Skip to content

fix(frequencies): define emptiness by stream weight - #281

Open
tisonkun wants to merge 1 commit into
apache:mainfrom
tisonkun:codex/frequencies-empty-semantics
Open

tisonkun wants to merge 1 commit into
apache:mainfrom
tisonkun:codex/frequencies-empty-semantics

Conversation

@tisonkun

Copy link
Copy Markdown
Member

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 returns false for 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 use num_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

  • Empty means zero total stream weight throughout the public API, serialization, and merge. Purged sketches retain the existing 32-byte representation with no items; reset sketches retain the 8-byte empty representation. The binary format is unchanged.
  • Updates, including borrowed-key updates, and merges check total-weight addition before mutation. Exceeding u64::MAX panics 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.
  • Deserialization rejects non-empty images with zero stream weight. It also rejects counter sums that exceed the declared stream weight, preventing overflow while reconstructing the map. Previously accepted inconsistent or wrapped images are no longer accepted.
  • Preamble length determines serialized emptiness and is checked against the empty flag. Both legacy flag bits remain accepted, individually or together.

Regression coverage exercises the 193-item scenario for both i64 and String, 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

  • Confirmed that the four new purged-state and round-trip cases fail against the previous implementation.
  • cargo x check
  • cargo x test
  • cargo x lint
  • cargo test --release --package tests-integration --test frequencies_test --test serde_tests weight

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant