Uh oh!
There was an error while loading. Please reload this page.
PYTHON-1369 Extend driver vector support to arbitrary subtypes and fix handling of variable length types (OSS C* 5.0) - #1217
Conversation
…borked for some reason but the other cases are now passing.
… vectors (or other types which might have non-standard representations)
| rv.append(abs(v)) | ||
| rv.reverse() | ||
| return bytes(rv) |
There was a problem hiding this comment.
Adapted from vints_pack and vints_unpack above. Those functions are different enough (built-in zig-zag encoding + tuples for incoming/outgoing data) that it seemed worthwhile to live with the code duplication here.
…an integration test) with PYTHON-1394.
| """ | ||
| The driver was unable to deserialize a given vector | ||
| """ | ||
| pass |
There was a problem hiding this comment.
Added in PYTHON-1371 to indicate an attempt to decode a vector of an unsupported subtype. With the other changes in this PR this exception is now completely unnecessary.
| @classmethod | ||
| def serial_size(cls): | ||
| serialized_size = cls.subtype.serial_size() | ||
| return cls.vector_size * serialized_size if serialized_size is not None else None |
There was a problem hiding this comment.
Here (as in many other points in this PR) we're following the example of what's done on the server side.
| def _normalize_set(self, val): | ||
| if isinstance(val, set) or isinstance(val, util.SortedSet): | ||
| return frozenset([self._normalize_set(v) for v in val]) | ||
| return val |
There was a problem hiding this comment.
Really just about making the types line up. The set codec returns a SortedSet by default and that class doesn't have all the niceties of frozenset so we convert here to simplify the equality comparison later.
absurdfarce
commented
Jul 24, 2024
Jenkins build at this point looks good: all failures appear to be known issues, most notably PYTHON-1390 and friends. |
…nv vars" This reverts commit 3363d16.
absurdfarce
commented
Aug 28, 2024
@SiyaoIsHiding the integration test is now complete and covers both tuples and UDTs. At this point I think I've covered everything we intended to cover so this PR should be done (plus or minus changes coming out of review). Would you mind taking another pass at this? |
| return "'%s'" % val.compressed | ||
| def cql_encode_decimal(self, val): | ||
| return self.cql_encode_float(float(val)) No newline at end of file |
There was a problem hiding this comment.
Fixing a bug discovered during testing. When using the drivers support for positional parameters in the execute() statement we weren't correctly expanding decimal types.
SiyaoIsHiding
left a comment
There was a problem hiding this comment.
Apart from the type mismatch error handling, looks good to me!
| idx += bytes_read | ||
| rv.append(cls.subtype.deserialize(byts[idx:idx + size], protocol_version)) | ||
| idx += size | ||
| return rv |
There was a problem hiding this comment.
Do we want to throw an error when the length of elements in the byts does not match the vector dimension definition? Same for serialize.
There was a problem hiding this comment.
This is actually happening (at least for deserialize) with the changes above. I've also recently added support for similar tests in serliaze() (along with tests for all the cases).
| lessthanorequalcass40 = unittest.skipUnless(CASSANDRA_VERSION <= Version('4.0-a'), 'Cassandra version less or equal to 4.0 required') | ||
| lessthancass40 = unittest.skipUnless(CASSANDRA_VERSION < Version('4.0-a'), 'Cassandra version less than 4.0 required') | ||
| greaterthanorequalcass40 = unittest.skipUnless(CASSANDRA_VERSION >= Version('4.0'), 'Cassandra version 4.0 or greater required') | ||
| greaterthanorequalcass50 = unittest.skipUnless(CASSANDRA_VERSION >= Version('5.0-beta'), 'Cassandra version 5.0 or greater required') |
There was a problem hiding this comment.
nit: as far as I know there isn't a branch called '5.0-beta' anymore. It's now called '5.0.0'.
There was a problem hiding this comment.
The intent here was to handle the various betas and release candidate releases as we run up to Cassandra 5.0. Once we get to an actual 5.0 release this can be easily changed.
absurdfarce
commented
Sep 3, 2024
I believe everything from the last go-round has been addressed. @SiyaoIsHiding would you mind taking another look? |
SiyaoIsHiding
commented
Sep 4, 2024
Sorry that previously I missed classCounterColumnType(LongType):
typename = 'counter'@classmethoddefserial_size(cls):
returnNoneI don't quite understand how |
…x handling of variable length types (OSS C* 5.0) (apache#1217)
…x handling of variable length types (OSS C* 5.0) (apache#1217)
…x handling of variable length types (OSS C* 5.0) (apache#1217)
…x handling of variable length types (OSS C* 5.0) (apache#1217)
…x handling of variable length types (OSS C* 5.0) (apache#1217)
…x handling of variable length types (OSS C* 5.0) (apache#1217)
No description provided.