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
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.
Summary
A purge can remove every counter while the sketch still carries stream weight and error bounds. For example, inserting 193 distinct items into a map of size 256 leaves zero active items, total weight 193, and maximum error 1.
is_empty()now returnsfalsefor this state, so callers do not mistake it for an unused sketch when deciding whether to retain or merge it. Callers that need the previous active-item semantics should usenum_active_items() == 0.Background
This follows the empty-state semantics proposed in apache/datasketches-java#770, alongside apache/datasketches-cpp#527 and apache/datasketches-cpp#529.
Rust already fixed the serialization and merge data loss in #191, released in 0.4.0. That change deliberately preserved the public active-item definition of
is_empty()and introduced a separate internal initial-state check. The internal check also protected state when a release-build weight overflow or an accepted inconsistent image left zero stream weight alongside counters or error bounds. This PR aligns the public API and addresses those two sources of zero-weight state before removing that separate check.Behavior and compatibility
u64::MAXpanics in every build profile and leaves the destination unchanged. This prevents release-build wrapping from making an updated sketch appear empty, without changing method signatures.Regression coverage exercises the 193-item scenario for both
i64andString, merging into empty and populated sketches, serialization round trips, reset, legacy flags, malformed weights, and overflow without mutation. Existing Java, C++, and Go serialization fixtures continue to pass. The new purged-state cases are generated locally; the pinned TCK revision is unchanged.Validation
cargo x checkcargo x testcargo x lintcargo test --release --package tests-integration --test frequencies_test --test serde_tests weight