Repository navigation
AoD tuple sketch: zero seed hash for empty compact images, ignored on read - #789
Merged
Merged
Conversation
Follow what Java theta does: an empty compact ArrayOfDoubles image carries a zero seed hash, and readers ignore those bytes on an empty image, so historical images that filled them in still load. Set operations skip the seed-hash check for empty inputs, as the theta set operations do. C++ already writes a zero seed hash for an empty compact tuple sketch (datasketches-cpp c11a5ea), which Java rejected in AodSketchCrossLanguageTest.checkCpp. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
proost
reviewed
Nov 2, 2026
| Util.checkSeedHashes(seedHash_, Util.computeSeedHash(seed)); | ||
| isEmpty_ = (seg.get(JAVA_BYTE, FLAGS_BYTE) & (1 << Flags.IS_EMPTY.ordinal())) != 0; | ||
| // the seed hash of an empty image is ignored; historical images may have filled it in | ||
| if (!isEmpty_) { Util.checkSeedHashes(seedHash_, Util.computeSeedHash(seed)); } |
Member
There was a problem hiding this comment.
To protect from corrupted input, can you change to (!isEmpty_ | hasEntries)?
Member
Author
There was a problem hiding this comment.
Good catch, thanks. Done in both the heap and direct readers (f63e080): the seed hash is now checked when the image is non-empty or has entries, so a corrupt image that claims to be empty but carries entries can't skip the check. Added a test with such a corrupt image. This is also a little stricter than C++, which checks when has-entries is set.
…tries An image is skipped by the seed hash check only when it is empty and has no entries. A corrupt image that sets both the empty and has-entries flags is still checked, so its entries can't bypass the check. Applies to the heap and direct readers. As suggested in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
An empty compact ArrayOfDoubles (AoD) image now carries a zero seed hash, and readers ignore those bytes on an empty image. This aligns AoD with Java Theta, with the generic tuple sketch (#772), and with C++ (datasketches-cpp c11a5ea), which already writes a zero seed hash for an empty compact tuple sketch.
Changes
HeapArrayOfDoublesCompactSketchandDirectArrayOfDoublesCompactSketchwrite a zero seed hash when the sketch is empty.ArrayOfDoublesUnion,ArrayOfDoublesIntersectionandArrayOfDoublesAnotBImplskip the seed-hash check for an empty input, as the Theta set operations do. A-not-B takes its seed hash from whichever input is non-empty.Why
Java rejected every empty AoD image written by C++ (and so by Python) with
Incompatible Seed Hashes. 0, 93cc.AodSketchCrossLanguageTest.checkCpphas been a known failure because of this; it passes with this change.Compatibility
Release 9.0.0 and earlier check the seed hash of an empty AoD image, so they reject an empty compact AoD image written by this version, just as they reject the C++ ones today. Non-empty AoD images are unchanged. This will be called out in the 9.1.0 release notes.
Testing
HeapArrayOfDoublesCompactSketchTestcover the zero seed hash on write, ignoring it on read, and reading a historical empty image with a filled-in seed hash. The union, intersection and A-not-B tests cover empty inputs.mvn verify: 2321 tests, 0 failures.AodSketchCrossLanguageTest.checkCppagainst the current C++ files: fails on main, passes with this change.🤖 Generated with Claude Code