-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add CRC64-AVRO-LE fingerprint type #491
Conversation
See also #490. |
c76586d
to
e126cff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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"? |
No stress, it will be documented in the release. I think that should be fine. |
Alright, I'll leave it to you. Thanks again. |
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.