DRIVERS-2926 BSON Binary Vector Subtype Support

[caseyclements]

Adds support for new subtype of Binary BSON type to be used for densely-packed 1d arrays (vectors).

Corresponding changes for the PyMongo as reference implementation are here.

DRIVERS-2926

[caseyclements]

Prose descriptions of tests that we suggest be added to each driver coming soon.

[caseyclements]

Rough draft of prose tests added.
Guidance would be great!

[jyemin]

This would benefit from a more precise set of test cases, and all the better if they are represented in YAML format like other unit tests in the specifications repository. I know that a set of tests have been implemented in pymongo, but they should all be specified here (i.e. an implementer shouldn't have to go spelunking in the pymongo source code to find all the necessary test cases).

There is no existing test format that does what we need, but I imagine a new set of tests could look something like this:

{
    "description: <some description>,
    "dtype" : "int8",
    "array" : [{ "$numberInt" : 42}, ...]
    "binaryAsBase64": "JwAAAP5CAADgQA==\"
}

[ShaneHarvey]

+1 to Jeff's suggestion for spec tests.

[ShaneHarvey]

This spec would be benefited by including a diagram or model of the format itself. For example the decimal128 spec defines "structs" to represent the data format.

It would also be great to include code examples in Python that show the expected use cases (python list -> bson vector and vice versa).

[caseyclements]

Happy to oblige, but I'm unsure what you have in mind for the model.

As to code examples, it was suggested to me not to include python code. There's now a link in the reference implementation. Sufficient?

[blink1073]

We can either add an image, e.g.

specifications/source/transactions/transactions.md

Line 261 in b5cbae0

<img src="client-session-transaction-states.png"

, or ascii art (

specifications/source/connection-string/connection-string-spec.md

Line 32 in b5cbae0

mongodb://username:password@example.com:27017,example2.com:27017,...,example.comN:27017/database?key=value&keyN=valueN

). Here's an example of some implementation suggestion code: https://github.com/mongodb/specifications/blob/b5cbae0211d3d5049680cfc0803e4589474464bc/source/auth/auth.md#hostname-canonicalization

[caseyclements]

This is over the top. How about an example?

For example, a vector [6, 7] of dtype PACKED_BIT (\x10) with a padding of 3 would like like this:
b"\x10\x03\x06\x07': 1 byte for dtype, 1 for padding, and 1 for each uint8.

[blink1073]

Why not write it as a markdown table and then explain the table?

[caseyclements]

@ShaneHarvey @blink1073 Please take a look now. I've added an html table.

[ShaneHarvey]

Please add a link to the PYTHON ticket.

[caseyclements]

I didn't see an example of a ticket, but I did add a link to the github module.

[caseyclements]

done.

[jyemin]

Looks good. Just gave a first pass to the test runner README.

[jyemin]

This file should describe all the assertions that the test runner should make. Many are obvious, but one that might be missed, for example, is that you can round trip from vector to bson and then back to vector.

IIRC the bson_corpus README does this pretty well.

[caseyclements]

I'm not sure if this is outdated now. If you need more, would you please be specific? I am also happy to revisit once another driver takes a stab at it.

[jyemin]

We discussed this in person, so I think we're in alignment now. The idea is to just look at all the assertions being done in the Python driver test runner, and write them down in prose here.

[caseyclements]

2d4ea72 - Added Required Tests section to README. Removed JSON from tests.

Review addressed

[blink1073]

This still needs updating

[blink1073]

"9"

[caseyclements]

Done.

[blink1073]

This file path might change, let's just use the full JIRA link to the ticket.

[caseyclements]

Done.

[vbabanin]

It seems that the server currently does not validate the metadata of vectors when they are inserted into the database. As a result, users can insert vectors with invalid metadata if they construct binary manually, such as setting the padding to -1 or applying padding to FLOAT32.

We do have tests in the specification that enforce validation in drivers' API to ensure that vectors with corrupted metadata never reach the server. However, the specification does not require drivers to validate vector metadata when the data is being read back from the database. This means that if the metadata becomes corrupted (e.g., a FLOAT32 vector with padding greater than 0), drivers could return a Vector object/structure/class from their API's to the user that contains invalid data and violates expected invariants.

Should we mention this in the spec API guidance and enforce validation when reading vectors from the database, ensuring that drivers check for invariant violations in the binary data? This would prevent returning invalid Vector structures to users.

[caseyclements]

@vbabanin This is a good idea. Would you like to propose some prose for this? How about the following?

Implementing drivers MUST validate metadata. Padding must be 0 for all dtypes where padding does not apply. When padding is applicable, it must be in [0, 7]. If implementing INT4 valid values would only be {0, 4}

[vbabanin]

That sounds good! I’d suggest making it just a bit more explicit to ensure clarity on where the validation should happen. How about something like this:

Drivers MUST validate vector metadata and raise an error if any invariant is violated:

• Padding MUST be 0 for all dtypes where padding doesn’t apply, and MUST be within [0, 7] for PACKED_BIT.
• A PACKED_BIT vector MUST NOT be empty if padding is in the range [1, 7].

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.

I didn’t include INT4 in validation requirements as it could be added later once it’s supported in the table of types.

What do you think?

[vbabanin]

Validation section was added in this commit: 0ccc39944

[vbabanin]

When converting a float32 which is already stored as an approximation, like 127.699..... for 127.7, in Java for example, to a 4-byte array and back, no additional approximation error should occur since it's a direct encoding and decoding of the binary representation. So precise matching in tests seems important for drivers that support float32 to catch any mistakes in the float to bytes & bytes to float algorithm itself. Should we consider mentioning this in the test guidance for drivers that natively support float32?

Should we also specify a small margin of error for comparing floating-point values for drivers supporting only float64 where precision loss could happen?

[caseyclements]

Something like this?

"For drivers that natively support types like float32, they SHOULD implement additional testing. For example, precise matching for drivers that support float32 to catch any mistakes in the float to bytes & bytes to float algorithm itself."

[vbabanin]

Sounds good! I think drivers that support float32 natively have to ensure precise conversion. How about extending it with the following wording?

Drivers that natively support the floating-point type being tested (e.g., when testing float32 vector values in a driver that natively supports float32), MUST assert that the input float array is the same after encoding and decoding.

What do you think?

[caseyclements]

I like it. I made an update. Please take a look.

[vbabanin]

That sounds good! I’d suggest making it just a bit more explicit to ensure clarity on where the validation should happen. How about something like this:

Drivers MUST validate vector metadata and raise an error if any invariant is violated:

• Padding MUST be 0 for all dtypes where padding doesn’t apply, and MUST be within [0, 7] for PACKED_BIT.
• A PACKED_BIT vector MUST NOT be empty if padding is in the range [1, 7].

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.

I didn’t include INT4 in validation requirements as it could be added later once it’s supported in the table of types.

What do you think?

[vbabanin]

Sounds good! I think drivers that support float32 natively have to ensure precise conversion. How about extending it with the following wording?

Drivers that natively support the floating-point type being tested (e.g., when testing float32 vector values in a driver that natively supports float32), MUST assert that the input float array is the same after encoding and decoding.

What do you think?

[vbabanin]

from_vector currently only accepts raw vector parameters, while as_vector returns a Vector structure. To keep the API consistent, it would make sense for from_vector to accept a Vector structure directly, similar to how as_vector returns one. This would also reduce the need for manually extracting individual components when encoding a Vector.

Should from_vector also accept a Vector type as input? Perhaps this could be offered as an alternative method for drivers to implement.

[caseyclements]

@vbabanin. I completely agree. That was an oversight. What about this?

The above is suggestive only. If a driver chooses to implement a Vector type (or numerous)
they MAY decide that from_vector that has a single argument, a Vector.

[nbbeeken]

LGTM!

[jyemin]

LGTM

[jyemin]

What can actually be asserted about floats? Can we place any sort of mathematical limit on the non-exact match?

[blink1073]

LGTM!