Skip to content

(improvement) query: add Cython-aware serializer path in BoundStatement.bind() (1-26x speedup - tens/hundreds of us reduced) - #749

Draft
mykaul wants to merge 4 commits into
scylladb:masterfrom
mykaul:perf/cython-bind-path
Draft

(improvement) query: add Cython-aware serializer path in BoundStatement.bind() (1-26x speedup - tens/hundreds of us reduced)#749
mykaul wants to merge 4 commits into
scylladb:masterfrom
mykaul:perf/cython-bind-path

Conversation

@mykaul

@mykaul mykaul commented Mar 14, 2026

Copy link
Copy Markdown

Summary

  • When Cython serializers (cassandra.serializers from PR (improvement) serializers: add Cython-optimized serialization for VectorType (up to 30x faster - 823us -> 4us!) #748) are available, there is no column encryption policy, and the statement contains at least one VectorType column, BoundStatement.bind() uses pre-built Serializer objects cached on the PreparedStatement instead of calling cqltype classmethods
  • The bind loop is split into three code paths: (1) column encryption policy, (2) Cython serializers (new fast path, vector-gated), (3) plain Python fallback
  • Pre-allocates self.values list to avoid repeated .append() growth

Dependencies

Update: fixed a gating regression found in review

The original version of this PR gated the Cython fast path on _HAVE_CYTHON_SERIALIZERS and self.column_metadata -- i.e. it used Cython serializers for every non-empty prepared statement whenever the Cython extension was available (which is nearly always in production, since pyproject.toml marks Cython extensions "must" for Linux/macOS wheels). But serializers.pyx's find_serializer() only has dedicated fast serializers for FloatType/DoubleType/Int32Type/VectorType; everything else goes through a GenericSerializer that just calls cqltype.serialize() anyway, but pays extra dispatch/caching overhead to get there. As the original benchmark below showed, that made ordinary scalar-only statements slower, not faster -- a real regression for the common case, contradicting the "falls back automatically" claim that was previously false.

Fix: PreparedStatement._serializers is now additionally gated on any(issubclass(col.type, VectorType) for col in self.column_metadata). Scalar-only statements now correctly fall straight through to the plain Python bind path; statements with at least one vector column (even if mixed with scalar columns) still get the Cython fast path for the whole statement, since per-column dispatch inside find_serializer() already picks the right serializer per column.

Also fixed in this update:

  • Reverted an unrelated if not objs: return objs early-return that had been added to the existing obj_array() in cassandra/deserializers.pyx. It didn't fix anything real (ParseDesc.deserializers is a typed Deserializer[::1] memoryview assigned unconditionally in ParseDesc.__init__, so an empty list still fails there, just one frame later) and this PR doesn't otherwise touch deserialization. The equivalent guard in the new serializers.pyx's own obj_array() is left in place (harmless, and currently unreachable since the only caller already guards on non-empty column_metadata), with a corrected docstring.
  • tests/unit/cython/test_serializers.py imported numpy unconditionally at module scope, which broke test collection on Cython-without-numpy builds. Now follows the existing repo convention (tests/integration/standard/test_cython_protocol_handlers.py): numpytest decorator on the 4 numpy-dependent tests, numpy imported lazily inside each.
  • Added tests exercising the gating logic directly (scalar-only -> _serializers is None, vector-containing -> _serializers is not None, plus no-Cython-available and empty-statement cases).

Benchmark

Measured with min() of timeit.repeat(repeat=7, number=50_000), BoundStatement.bind() end-to-end.

Original numbers (unconditional Cython gating, before this update's fix):

Workload Python fallback (ns/call) Cython path (ns/call) Speedup
Vector<float, 128> (1 col) 11460 2847 4.0x
Vector<float, 1536> (1 col) 112896 10397 10.9x
10 mixed scalars (5 float + 3 int32 + 2 text) 2174 3748 0.6x (regression)

After the gating fix (different machine, same methodology -- absolute numbers aren't directly comparable across machines, but the shape of the result is what matters):

Workload Path taken ns/call
10 mixed scalars -- old unconditional gating (forced Cython) Cython ~4400-4600
10 mixed scalars -- new gating (default) Python (_serializers is None) ~2300-2800
Vector<float, 128> (1 col) -- new gating (default) Cython ~3100-3500
Vector<float, 128> (1 col) -- forced Python Python ~18200
Vector<float, 1536> (1 col) -- new gating (default) Cython ~14500
Vector<float, 1536> (1 col) -- forced Python Python ~196800

So: the scalar-only regression is gone (statement now takes the Python path automatically, ~1.7-2x faster than the old forced-Cython behavior for that workload), and the vector speedup (roughly 5-13x here) is fully preserved.

Key observations:

  • Vector columns get significant speedups because the Cython serializer replaces the per-element io.BytesIO loop with a pre-allocated C buffer and inline byte-swap
  • Scalar-only workloads now correctly use the plain Python path -- _serializers returns None unless the statement contains at least one VectorType column, so mixed-scalar workloads fall back to the Python path automatically
  • Speedup scales with vector dimension

Tests

  • tests/unit/test_parameter_binding.py: comprehensive bind path tests covering Cython path, Python fallback, error wrapping, UNSET_VALUE handling, and the _serializers gating logic itself
  • tests/unit/cython/test_serializers.py: Cython serializer equivalence tests, including numpy-array-input tests now properly gated behind @numpytest
  • Full unit test suite passes (785 passed, 88 skipped)

This remains a draft / experimental PR pending further review.

@mykaul
mykaul marked this pull request as draft March 14, 2026 11:22
@mykaul
mykaul requested a review from Copilot March 14, 2026 19:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an optional, Cython-accelerated serialization path for BoundStatement.bind() to reduce per-value overhead (especially for large VectorType columns) when Cython serializers are available and no column encryption policy is enabled.

Changes:

  • Add a new Cython cassandra.serializers extension (with .pyx + .pxd) providing Serializer implementations and a make_serializers() factory.
  • Add lazy caching of per-column serializer objects on PreparedStatement via a _serializers property.
  • Split the bind loop into three branches: column encryption policy, Cython fast path, and pure-Python fallback (with reduced per-value overhead in the fallback).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
cassandra/serializers.pyx Adds Cython serializer implementations (scalar + VectorType) and lookup/factory functions.
cassandra/serializers.pxd Exposes the Serializer cdef interface for Cython usage.
cassandra/query.py Integrates the optional Cython serializer path into PreparedStatement/BoundStatement.bind() with lazy caching and branch selection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/serializers.pyx Outdated
Comment thread cassandra/serializers.pyx
Comment thread cassandra/query.py Outdated
Comment thread cassandra/query.py Outdated
Comment thread cassandra/query.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an optimized parameter binding path that can leverage Cython Serializer objects (when available) to speed up BoundStatement.bind(), while preserving the existing column-encryption behavior and improving the plain-Python fallback.

Changes:

  • Add PreparedStatement._serializers lazy cache and a three-way bind loop in BoundStatement.bind() (CE policy / Cython fast path / Python fallback).
  • Introduce Cython Serializer implementations for Float/Double/Int32/Vector and a make_serializers() factory.
  • Extend unit tests to exercise the Cython-serializer bind branch via injected stub serializers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
cassandra/query.py Adds Cython-serializer availability detection, caches serializers on PreparedStatement, and selects the new bind fast path when safe.
cassandra/serializers.pyx Implements Cython serializers (including optimized VectorType) and factory/lookup helpers.
cassandra/serializers.pxd Declares the Cython Serializer interface for cross-module typing.
tests/unit/test_parameter_binding.py Adds tests for the new bind branch and error-wrapping behavior using stub serializers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/serializers.pyx Outdated
Comment thread tests/unit/test_parameter_binding.py Outdated
Comment thread cassandra/serializers.pyx
Comment thread cassandra/serializers.pyx

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Cython-aware fast path to BoundStatement.bind() so prepared statements can use cached per-column serializer objects (when available and no column encryption policy is active), reducing Python dispatch and per-value overhead during binding.

Changes:

  • Add lazy PreparedStatement._serializers cache and split BoundStatement.bind() into CE-policy, Cython-serializer, and pure-Python paths.
  • Introduce Cython Serializer implementations (including optimized VectorType serialization) and a make_serializers() factory.
  • Expand/adjust unit tests to cover the new Cython bind branch via injected stub serializers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
cassandra/query.py Adds serializer caching and a new bind fast path (plus significant formatting-only edits in the same module).
cassandra/serializers.pyx Introduces Cython serializer implementations and factory/lookup helpers.
cassandra/serializers.pxd Declares the Cython Serializer interface for cross-module use.
tests/unit/test_parameter_binding.py Adds tests to exercise the Cython bind path without requiring compiled Cython.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/serializers.pyx Outdated
Comment thread cassandra/query.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Cython-aware fast path to BoundStatement.bind() by reworking the bind loop into distinct branches (column encryption vs Cython serializers vs pure-Python), and introduces the Cython serializer module used by the new path.

Changes:

  • Add lazy cached PreparedStatement._serializers and update BoundStatement.bind() to use Cython Serializer objects when available and CE policy is not active.
  • Factor bind-time serialization error wrapping into a shared helper and extend wrapping to include OverflowError.
  • Add unit tests that exercise the new Cython bind branch via injected stub serializers, plus overflow wrapping coverage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
cassandra/query.py Adds _serializers cache on PreparedStatement, a shared bind error wrapper, and splits BoundStatement.bind() into CE / Cython / Python paths.
cassandra/serializers.pyx Introduces Cython Serializer implementations (scalar + vector) and serializer factory helpers.
cassandra/serializers.pxd Exposes the Serializer cdef interface for Cython interop.
tests/unit/test_parameter_binding.py Adds unit tests to validate the new Cython bind path behavior and error wrapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/query.py Outdated
Comment thread cassandra/serializers.pyx Outdated
Comment thread cassandra/serializers.pyx

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an optimized binding path that leverages Cython-backed per-column Serializer objects (when available) to reduce Python overhead in BoundStatement.bind(), while keeping existing behavior for column encryption policy and providing a streamlined pure-Python fallback.

Changes:

  • Add lazy, cached Cython serializer lookup on PreparedStatement and route BoundStatement.bind() through a 3-way path: CE-policy, Cython fast path, pure-Python fallback.
  • Introduce Cython serializer implementations (Serializer, scalar serializers, SerVectorType, and factories).
  • Extend unit tests to exercise the Cython fast-path behavior (via injected stub serializers) and UNSET handling scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
cassandra/query.py Adds optional Cython serializer import, caches per-column serializers on PreparedStatement, and refactors BoundStatement.bind() into CE/Cython/Python paths with pre-allocation.
cassandra/serializers.pyx Introduces Cython-optimized serializers (including VectorType) and factory functions for per-column serializer creation.
cassandra/serializers.pxd Declares the Cython Serializer interface for cross-module typing/cimports.
tests/unit/test_parameter_binding.py Adds tests that inject stub serializers to validate the Cython bind path and UNSET behavior across paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/serializers.pyx
Comment thread cassandra/query.py Outdated
Comment thread cassandra/query.py Outdated
@mykaul
mykaul force-pushed the perf/cython-bind-path branch from 9aabe6b to 414f0cd Compare April 3, 2026 18:12
@mykaul mykaul changed the title (improvement) query: add Cython-aware serializer path in BoundStatement.bind() (improvement) query: add Cython-aware serializer path in BoundStatement.bind() (1-26x speedup - tens/hundreds of us reduced) Apr 7, 2026
@mykaul
mykaul force-pushed the perf/cython-bind-path branch 2 times, most recently from ee511ea to 37a597b Compare April 7, 2026 09:22
@mykaul
mykaul requested a review from Copilot July 26, 2026 06:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

cassandra/serializers.pyx:251

  • This indexability probe does not preserve VectorType.serialize()'s iteration semantics. For example, a dict {0: 10, 1: 20, 2: 30} previously serializes the iterated keys (0, 1, 2), but this branch sees value[0] succeed and serializes the mapped values (10, 20, 30) instead. After the buffer fast paths, normalize every non-list/tuple iterable before the indexed loops (or otherwise guarantee indexing and iteration are equivalent).
        if v_length != 0 and not isinstance(value, (list, tuple)):
            try:
                value[0]
            except (TypeError, KeyError, IndexError):
                value = tuple(value)

Comment thread cassandra/deserializers.pyx Outdated
Comment thread cassandra/query.py Outdated
Comment thread tests/unit/cython/test_serializers.py Outdated
Copilot AI review requested due to automatic review settings July 29, 2026 16:05
@mykaul
mykaul force-pushed the perf/cython-bind-path branch from 37a597b to a846c5a Compare July 29, 2026 16:05
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 805a7226-8890-4c11-b468-da20caed6cdf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@mykaul

mykaul commented Jul 29, 2026

Copy link
Copy Markdown
Author

Pushed a follow-up commit (query: gate Cython serializer fast path on VectorType presence to avoid scalar regression) addressing the review feedback, rebased onto current master:

  1. Core fix: PreparedStatement._serializers was gating on _HAVE_CYTHON_SERIALIZERS and self.column_metadata alone, which meant every non-empty prepared statement used the Cython fast path whenever the extension was built -- including the scalar-only case this PR's own benchmark showed was a ~72% regression (3748ns vs 2174ns). Now additionally gated on any(issubclass(col.type, VectorType) for col in self.column_metadata). Scalar-only statements correctly fall through to the plain Python path now; vector-containing statements (even mixed with scalars) still get the fast path.
  2. Reverted the unrelated obj_array() early-return in cassandra/deserializers.pyx -- it didn't fix anything real (the typed-memoryview assignment in ParseDesc.__init__ still fails on empty input regardless) and this PR doesn't otherwise touch deserialization. The equivalent guard in the new serializers.pyx::obj_array() stays (harmless/defensive, currently unreachable), with a docstring that no longer overclaims.
  3. Fixed the unconditional import numpy as np at module scope in tests/unit/cython/test_serializers.py, which broke test collection on Cython-without-numpy builds. Now uses @numpytest on the 4 numpy-dependent tests with lazy imports, matching the existing convention in test_cython_protocol_handlers.py.
  4. Added tests directly exercising the gating logic (SerializersGatingTest in tests/unit/test_parameter_binding.py): scalar-only -> _serializers is None, vector-containing -> _serializers is not None, plus no-Cython and empty-statement cases.
  5. Updated the PR description with the corrected gating behavior and rough before/after numbers confirming the scalar regression is gone while the vector speedup is preserved.

Full unit test suite: 785 passed, 88 skipped, 0 failed (built with CASS_DRIVER_BUILD_EXTENSIONS_ARE_MUST=yes python setup.py build_ext --inplace --no-murmur3 --no-libev, ran with and without VERIFY_CYTHON=1).

Resolved the three review threads this addresses (deserializers.pyx obj_array, query.py unconditional gating, test_serializers.py numpy import).

Still a draft/experiment -- not proposing this for merge yet.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

cassandra/serializers.pyx:251

  • The Python implementation serializes by iterating the input, but this probe switches to integer indexing whenever value[0] happens to succeed. Inputs whose iteration and indexing differ then change behavior; for example, an integer-keyed mapping is serialized as its keys by VectorType.serialize() but as its mapped values here. Preserve iteration semantics after the buffer paths miss (for example, by normalizing before the specialized loops or making those loops consume the iterator directly).
        if v_length != 0 and not isinstance(value, (list, tuple)):
            try:
                value[0]
            except (TypeError, KeyError, IndexError):
                value = tuple(value)

cassandra/serializers.pyx:186

  • These helpers classify every subclass of the built-in scalar types as safe for raw C encoding. For a custom subtype that overrides serialize(), VectorType.serialize() uses that override, but this fast path silently bypasses it and emits built-in float/double/int32 bytes instead. Restrict specialization to the exact built-in classes (consistent with the scalar factory's exact-name lookup) and let subclasses use the generic path.
    return subtype is cqltypes.FloatType or issubclass(subtype, cqltypes.FloatType)

mykaul added 3 commits July 30, 2026 12:49
…torType

Add cassandra/serializers.pyx and cassandra/serializers.pxd implementing
Cython-optimized serialization that mirrors the deserializers.pyx architecture.

Implements type-specialized serializers for the three subtypes commonly used
in vector columns:
- SerFloatType: 4-byte big-endian IEEE 754 float
- SerDoubleType: 8-byte big-endian double
- SerInt32Type: 4-byte big-endian signed int32

SerVectorType pre-allocates a contiguous buffer and uses C-level byte swapping
for float/double/int32 vectors, with a generic fallback for other subtypes.
GenericSerializer delegates to the Python-level cqltype.serialize() classmethod.

Factory functions find_serializer() and make_serializers() allow easy lookup
and batch creation of serializers for column types.

Benchmarks show ~30x speedup over the current io.BytesIO baseline and ~3x
speedup over Python struct.pack for Vector<float, 1536> serialization.

No setup.py changes needed - the existing cassandra/*.pyx glob already picks
up new .pyx files.
…nt.bind()

When Cython serializers (from cassandra.serializers) are available and no
column encryption policy is active, BoundStatement.bind() now uses
pre-built Serializer objects cached on the PreparedStatement instead of
calling cqltype classmethods. This avoids per-value Python method dispatch
overhead and enables the ~30x vector serialization speedup from the Cython
serializers module.

The bind loop is split into three paths:
1. Column encryption policy path (unchanged behavior)
2. Cython serializers path (new fast path)
3. Plain Python path (no CE, no Cython -- removes per-value ColDesc/CE check)

Depends on PR scylladb#748 (Cython serializers module) and PR scylladb#630 (CE-policy
bind split).
Copilot AI review requested due to automatic review settings July 30, 2026 09:59
@mykaul
mykaul force-pushed the perf/cython-bind-path branch from a846c5a to 008998f Compare July 30, 2026 09:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

cassandra/serializers.pyx:548

  • Dispatching solely by __name__ can select a built-in serializer for a different registered class named FloatType, DoubleType, or Int32Type, silently bypassing that class's serialization contract. Key the lookup by the actual built-in type objects (or verify exact identity), not class-name strings.
    # For scalar types with dedicated serializers, look up by name
    name = 'Ser' + cqltype.__name__
    cls = _ser_classes.get(name)
    if cls is not None:
        return cls(cqltype)

cassandra/serializers.pyx:186

  • The subclass checks assume every registered subtype subclass has the base type's wire format. CassandraTypeType registers custom subclasses, and such a subclass may override serialize(); a vector using it would now bypass that override and emit built-in float/double/int32 bytes. Select the specialized path only when the nearest class defining serialize is the corresponding built-in type; otherwise use the generic path.

This issue also appears on line 544 of the same file.

cdef inline bint _is_float_type(object subtype):
    return subtype is cqltypes.FloatType or issubclass(subtype, cqltypes.FloatType)

cassandra/serializers.pyx:542

  • This also routes every parameterized VectorType subclass through SerVectorType, bypassing a custom subclass's overridden serialize() method. Preserve the optimization for normal generated vector classes, but return GenericSerializer when the nearest MRO class defining serialize is not the built-in VectorType.
    if issubclass(cqltype, cqltypes.VectorType):
        if getattr(cqltype, 'subtype', None) is not None:
            return SerVectorType(cqltype)

…id scalar regression

PreparedStatement._serializers previously built and used Cython serializers
for every non-empty prepared statement whenever the Cython extension was
available. The PR's own benchmark showed this is a regression for ordinary
scalar-only statements (~1.7-2x slower here) because serializers.pyx only has
dedicated fast serializers for FloatType/DoubleType/Int32Type/VectorType --
everything else goes through GenericSerializer, which just calls
cqltype.serialize() anyway but pays extra dispatch overhead to get there. The
real win (multiple times faster) is specific to VectorType columns.

Gate _serializers on the statement containing at least one VectorType column,
so scalar-only statements now correctly fall through to the plain Python bind
path (matching the "falls back automatically" claim in the PR description),
while vector-containing statements still get the fast path.

Also:
- Revert the unrelated obj_array() early-return added to deserializers.pyx.
  ParseDesc.deserializers is a typed `Deserializer[::1]` memoryview assigned
  unconditionally in ParseDesc.__init__, so an empty list still crashes there
  regardless; the guard didn't fix anything and this PR doesn't otherwise
  touch deserialization. Fix the docstring on the equivalent (harmless,
  currently unreachable) guard in serializers.pyx's own obj_array() so it
  doesn't overclaim end-to-end safety.
- Fix tests/unit/cython/test_serializers.py importing numpy unconditionally
  at module scope, which breaks test collection on Cython-without-numpy
  builds. Follow the existing convention (see
  test_cython_protocol_handlers.py): import numpytest alongside cythontest,
  decorate the 4 numpy-dependent tests with it, and import numpy lazily
  inside each of those tests.
- Add tests exercising the new gating logic directly: scalar-only statements
  get _serializers is None; VectorType-containing statements get
  _serializers is not None; no-Cython-available and empty-statement cases
  are also covered.
- Guard SerializersGatingTest with @cythontest (tests/unit/cython/utils.py).
  Its tests monkeypatch cassandra.query._cython_make_serializers, a
  module-level name that only exists when `from cassandra.serializers
  import make_serializers` succeeds; on PyPy wheels (setup.py disables
  try_cython there) that import never happens, so mock.patch.object raised
  AttributeError instead of skipping -- the same crash class already fixed
  for CythonParserTest in test_protocol.py. Verified the tests now SKIP
  cleanly (instead of crashing) when the compiled Cython extensions are
  absent, and still PASS when they are present.
- Fix serializers.pyx dispatch to no longer bypass a user-defined subclass's
  overridden serialize(). find_serializer()'s scalar path matched by
  'Ser' + cqltype.__name__ (a same-named-but-different class would silently
  get the built-in packer), and both find_serializer()'s VectorType check and
  the _is_float_type()/_is_double_type()/_is_int32_type() helpers used plain
  is/issubclass against the built-in FloatType/DoubleType/Int32Type/VectorType
  (any subclass matches, including one overriding serialize()). This is a
  real, reachable gap, not just theoretical: CassandraTypeType auto-registers
  any non-underscore-prefixed class by its own __name__ purely as a side
  effect of the class statement executing (see
  tests/unit/test_types.py::test_parse_casstype_args for the same pattern
  applied to the generic CassandraType base), so a subclass with an
  overridden serialize() -- intentional or an accidental name clash -- can
  end up as a real column's type or a vector's subtype, at which point the
  Cython fast path would silently emit the built-in's hard-coded bytes
  instead of calling the override, corrupting the wire format without any
  error. Add _nearest_serialize_owner(), which walks the MRO to find the
  class that actually defines serialize() (apply_parameters() only ever adds
  data attributes like subtypes/vector_size/fieldnames, never serialize, so
  this reliably distinguishes a plain generated subtype from one with a real
  override), and use it everywhere this dispatch decides whether to use a
  built-in Ser* class. Keyed _ser_classes by the built-in type objects
  themselves rather than name strings. Added regression tests
  (TestFindSerializerRespectsSerializeOverride) covering scalar and vector
  subclass overrides, including a vector whose subtype (not the vector class
  itself) overrides serialize(), plus a test confirming a non-overriding
  subclass still takes the fast path so the optimization isn't regressed.
  This dispatch only runs once per PreparedStatement (inside
  make_serializers(), which is cached), so the fix has no effect on the
  per-value serialize() hot path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mykaul
mykaul force-pushed the perf/cython-bind-path branch from 008998f to 683ab06 Compare July 30, 2026 11:04
Copilot AI review requested due to automatic review settings July 30, 2026 11:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

cassandra/serializers.pyx:116

  • This converts a float-like object twice: once for the range check and again for the value being packed. Objects implementing __float__ may be stateful, so this can serialize a different value than FloatType.serialize()/struct.pack(), which coerces once. Cache the double conversion and cast that cached value to float.
    cpdef bytes serialize(self, object value, int protocol_version):
        _check_float_range(<double>value)
        cdef float val = <float>value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants