Report a zero seed hash for an empty compact sketch - #518
Merged
Conversation
An empty sketch retains no hashes, so its seed hash carries no
information and must not be validated. Deserialization
(deserialize_v3/deserialize_v4), theta_union_base::update() and
theta_intersection_base::update() all already skip the seed hash check
for empty inputs, and the existing seed mismatch tests record the intent
with the comment "non-empty should not be ignored".
theta_set_difference_base::compute() was the one path that still checked
it. When A is non-empty with zero retained entries, the early return does
not fire, and an empty B whose seed hash differs makes a-not-b throw
"B seed hash mismatch" where union and intersection accept the same pair.
This also breaks Java/C++ interop: datasketches-java serializes every
empty compact sketch as the constant {1,3,3,0,0,0x1E,0,0}, with a seed
hash of 0 that is documented as ignored, so any empty sketch coming from
Java hits this path.
The behavior is reproducible in C++ alone using two different seeds.
Generated-by: Claude Code (Claude Opus 4.8)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Java EmptyCompactSketch.getSeedHash() returns 0 and its serialized form carries zeros in that position, with the recognition mask excluding those two bytes. C++ carried compute_seed_hash(seed) instead, so an empty sketch did not round trip to the same bytes as Java. Generated-by: Claude Code
leerho
approved these changes
Aug 31, 2026
leerho
left a comment
leerho
left a comment
Member
There was a problem hiding this comment.
Again, thank you for your thoroughness! I look forward to more contributions from you!
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
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.
Follow-up to #517, which this is stacked on. It is the write half of what @leerho described there: make an empty compact sketch report a zero seed hash so it matches Java.
EmptyCompactSketchin Java does two things.getSeedHash()returns0, and the serialized singleton is{ 1, 3, 3, 0, 0, 0x1E, 0, 0 }with zeros in the seed hash position. The recognition mask0X00_00_EB_00_00_FF_FF_FFhas00_00in the top two bytes, so those bytes are excluded from the test rather than merely conventionally ignored.C++ carried
compute_seed_hash(seed)for an empty compact sketch, so the bytes differed from Java. After this an empty sketch serializes to1 3 3 0 0 30 0 0, the same eight bytes Java writes.Three accessors carry it, since making it accessor level rather than serialization level is what matches Java:
compact_theta_sketch_alloc,wrapped_compact_theta_sketch_alloc, andcompact_tuple_sketch.Why it is stacked rather than standalone. On its own this is a regression.
theta_set_difference_base::computechecks B's seed hash, and its early return only covers an empty B when A has retained entries, so an A that is non-empty with zero retained reaches the check. Today both sides carry the same computed hash and it passes; with this change B would carry 0 and it would throw. I confirmed that on a real sketch rather than reasoning about it:#517 adds the
!b.is_empty()guard that closes it, which is why this sits on top. Please take #517 first.The other three seed checks were already safe:
theta_union_base::updatereturns early on empty,theta_intersection_base::updatehas!sketch.is_empty() &&, anddeserialize_v3hasif (!is_empty) check_seed_hash(...), so a zero hash on an empty image reads back fine.deserialize_v1/v2check unconditionally, but those parse SerVer 1 and 2 images and nothing writes an empty sketch in those formats here.All 17 ctest suites pass, including the theta and tuple cross-language serde tests.
Generated with Claude Code; I ran the checks above myself.