Skip to content

fix(countmin): add portable serial version 2 - #299

Draft
tisonkun wants to merge 1 commit into
apache:mainfrom
tisonkun:fix/countmin-portable-hashing
Draft

tisonkun wants to merge 1 commit into
apache:mainfrom
tisonkun:fix/countmin-portable-hashing

Conversation

@tisonkun

Copy link
Copy Markdown
Member

Summary

This draft implements the rough consensus from the TCK discussion for cross-project review.

  • Add Count-Min serial version 2. Derive row seeds as XXH64(row_index.to_le_bytes(), base_seed) and use XXH64 for item hashes.
  • Map version 2 item hashes to buckets with (hash >> 1) % num_buckets.
  • Preserve the legacy Rust version 1 hash path when reading, updating, and reserializing old images.
  • Reject merges between different serial versions and document the remaining cross-language version 1 limitations.
  • Add independently computed version 2 known-answer vectors and legacy Rust version 1 regression coverage.

Compatibility

New sketches serialize as version 2. Legacy Rust version 1 images remain readable and retain version 1 when reserialized, so their counters are not mislabeled as using the new hash scheme.

Version 1 images from other implementations remain unreliable for queries, updates, and merges because the image does not identify their legacy row-seed derivation. Existing C++ version 1 snapshots continue to be covered for deserialization and byte round-tripping.

Cross-language version 2 TCK snapshots and byte comparisons still need to be added after the format details are agreed across implementations.

Refs: apache/datasketches-tck#18
Refs: #297

Validation

  • cargo x check
  • cargo x prepare-testdata
  • cargo x test
  • cargo x lint

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