DRIVERS-3123 Relaxes requirement on reads of the ignored bits of PACKED_BIT vectors. #1812
Conversation
…he given padding (those ignored) must be zero.
…s in bson binary vectors
…D_BIT with inconsistent padding
| as all of its bits are ones. | ||
|
|
||
| #### 1. Encoding | ||
|
|
There was a problem hiding this comment.
| Test encoding with non-zero ignored bits. Use the driver API that validates vector metadata. |
Suggest clarifying to test the driver API with validation. I expect encoding can refer to many types: EJSON, BSON, bson.binary.Binary, bson.binary.BinaryVector:
Drivers MUST perform this validation when a numeric vector and padding are provided through the API, and when unpacking binary data (BSON or similar) into a Vector structure.
There was a problem hiding this comment.
@kevinAlbs Have a look again. I've added your suggestions, but not specified the many types that you rightly point out encoding could refer to. I was always assuming the bson binary with subtype 9.
|
|
||
| #### 1. Encoding | ||
|
|
||
| - Test that an exception is raised when one attempts to encode a vector with non-zero ignored bits. |
There was a problem hiding this comment.
Since drivers may have differing behavior (to preserve backwards compat), suggest noting expectation may differ.
| - Test that an exception is raised when one attempts to encode a vector with non-zero ignored bits. | |
| If the driver validates ignored bits are zero (preferred), expect an error. Otherwise expect the ignored bits are preserved. |
| - Test that one can manually create a BSON Binary object following the vector structure (dtype + padding + data). | ||
| - Test that this can be inserted and found |
There was a problem hiding this comment.
| - Test that one can manually create a BSON Binary object following the vector structure (dtype + padding + data). | |
| - Test that this can be inserted and found |
Consider removing steps to insert+find. I expect this test is intended to only test decoding.
|
|
||
| - Test that one can manually create a BSON Binary object following the vector structure (dtype + padding + data). | ||
| - Test that this can be inserted and found | ||
| - Test the behaviour of your driver when one attempts to decode from binary to vector. |
There was a problem hiding this comment.
Consider adding a statement to "Skip this test if your driver does not provide a Vector type". I expect a vector type is not required:
If a driver chooses to implement a
Vectortype
Similar suggestion applies to the "Comparison" test.
| v2 = Binary.as_vector(b2) | ||
|
|
||
| assert b1 != b2 # Unequal at naive Binary level | ||
| assert v2 !=v1 # Also chosen to be unequal at BinaryVector level as [255] != [128] |
There was a problem hiding this comment.
| assert v2 !=v1 # Also chosen to be unequal at BinaryVector level as [255] != [128] | |
| assert v2 != v1 # Also chosen to be unequal at BinaryVector level as [255] != [128] |
| server does. | ||
|
|
||
| ```python | ||
| b1 = Binary.from_vector([0b10000000], BinaryVectorDtype.PACKED_BIT, padding=7) |
There was a problem hiding this comment.
IIUC this example is showing comparison between two bson.binary.Binary types. Suggest changing to compare two bson.binary.BinaryVector types?
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).