jaideeppyne opened a new pull request, #518:
URL: https://github.com/apache/datasketches-cpp/pull/518
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.
`EmptyCompactSketch` in Java does two things. `getSeedHash()` returns `0`,
and the serialized singleton is `{ 1, 3, 3, 0, 0, 0x1E, 0, 0 }` with zeros in
the seed hash position. The recognition mask `0X00_00_EB_00_00_FF_FF_FF` has
`00_00` in 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 to `1 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`, and `compact_tuple_sketch`.
**Why it is stacked rather than standalone.** On its own this is a
regression. `theta_set_difference_base::compute` checks 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:
```
A: is_empty=0 retained=0 seed_hash=37836
B: is_empty=1 seed_hash=37836
early return taken? no
a_not_b: OK
```
#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::update`
returns early on empty, `theta_intersection_base::update` has
`!sketch.is_empty() &&`, and `deserialize_v3` has `if (!is_empty)
check_seed_hash(...)`, so a zero hash on an empty image reads back fine.
`deserialize_v1`/`v2` check 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
