DRIVERS-3123 Relaxes requirement on reads of the ignored bits of PACKED_BIT vectors. #1812
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…he given padding (those ignored) must be zero.
…s in bson binary vectors
…D_BIT with inconsistent padding
nbbeeken
reviewed
Jun 25, 2025
kevinAlbs
reviewed
Jun 25, 2025
kevinAlbs
reviewed
Jun 27, 2025
kevinAlbs
approved these changes
Jun 30, 2025
| assert v2 != v1 # Also chosen to be unequal at BinaryVector level as [255] != [128] | ||
| ``` | ||
|
|
||
| Drivers MAY skip this test if they choose not to implement a `Vector` type. |
There was a problem hiding this comment.
Suggested change
| Drivers MAY skip this test if they choose not to implement a `Vector` type. | |
| Drivers MAY skip this test if they choose not to implement a `Vector` type, or the type does not support comparison. |
Suggest permitting drivers skip this test if the Vector type does not support comparison. The C driver bson_vector_packed_bit_view_t (which I expect is the closest analog to the Vector type) does not support comparison.
| assert b1 == Binary(b'\x10\x07\x80', subtype=9) # This is effectively a roundtrip. | ||
| v1 = Binary.as_vector(b1) | ||
|
|
||
| b2 = Binary.from_vector([0b11111111], BinaryVectorDtype.PACKED_BIT, padding=7) |
There was a problem hiding this comment.
I expect Binary.from_vector would raise an exception for non-zero ignored bits. Suggest instead directly doing:
b1 = Binary(b'\x10\x07\x80', subtype=9)
b2 = Binary(b'\x10\x07\xff', subtype=9)
nbbeeken
approved these changes
Jun 30, 2025
There was a problem hiding this comment.
LGTM, however possibly another driver maintainer is worth looping in (up to you @caseyclements)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have chosen not to require drivers to throw an error if data coming out of the server has non-zero ignored bits. This is for backwards-compatibility. The changes are the result of conversations with Dev Tools. The situation we want to avoid is that a non-technical user cannot get their data out. Imagine the situations of a user of Compass, who tries to look at her data, and gets nothing but exceptions. Instead, we require new writes to zero our data on writes, but allow freedom for drivers to migrate to the end goal of always requiring ignored bits to be zero according to their particular versioning and whether or not they've already implemented this feature.
Completed:
clusters).