feat: add CRC64-AVRO-LE fingerprint type #491
Merged
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
Contributor
Author
|
See also #490. |
nrwiersma
reviewed
Jan 20, 2025
kimgr
force-pushed
the
sum-encoding
branch
3 times, most recently
from
c76586d to
e126cff
Compare
nrwiersma
reviewed
Jan 23, 2025
Member
nrwiersma
left a comment
nrwiersma
left a comment
There was a problem hiding this comment.
One minor issue, otherwise it is looking good.
The Avro specification details a Single Object Encoding using a header to associate a schema ID with an Avro payload. The ID is defined as the CRC64 fingerprint in little-endian encoding. The pkg/crc64 module only provides big-endian CRC64, and the CRC64-AVRO fingerprint type is implemented as such. The specification does not detail endianness of the CRC64-AVRO fingerprint itself (only when embedded in an SOE header). To avoid breaking existing CRC64-AVRO fingerprints, add a new fingerprint type CRC64-AVRO-LE, identical to CRC64-AVRO except little-endian. Add NewWithByteOrder and SumWithByteOrder top-level functions to crc64 so users can configure the hasher to use a specific byte order. Add tests and benchmarks for the SumWithByteOrder function. Fixes hamba#489.
Contributor
Author
|
Thanks! Oh, I forgot about the README in all the revisions. Were you thinking something like "if you want to use a schema fingerprint for SOE, use CRC64-AVRO-LE because reasons"? |
Member
|
No stress, it will be documented in the release. I think that should be fine. |
Contributor
Author
|
Alright, I'll leave it to you. Thanks again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The Avro specification details a Single Object Encoding using a header
to associate a schema ID with an Avro payload. The ID is defined as the
CRC64 fingerprint in little-endian encoding.
The pkg/crc64 module only provides big-endian CRC64, and the CRC64-AVRO
fingerprint type is implemented as such. The specification does not
detail endianness of the CRC64-AVRO fingerprint itself (only when
embedded in an SOE header).
To avoid breaking existing CRC64-AVRO fingerprints, add a new
fingerprint type CRC64-AVRO-LE, identical to CRC64-AVRO except
little-endian.
Add NewWithByteOrder and SumWithByteOrder top-level functions to crc64
so users can configure the hasher to use a specific byte order.
Add tests and benchmarks for the SumWithByteOrder function.
Fixes #489.