From e99211eab560133089442a5c8a495d09ec9888f0 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 22 Oct 2026 15:48:39 +0800 Subject: [PATCH 01/13] fix(countmin): add portable v2 hashing --- CHANGELOG.md | 1 + datasketches/src/countmin/serialization.rs | 3 +- datasketches/src/countmin/sketch.rs | 97 +++++++++++++------ datasketches/src/hash/mod.rs | 6 +- datasketches/src/hash/xxhash.rs | 10 ++ .../tests/countmin_test/sketch.rs | 49 ++++++++++ .../tests/serde_tests/countmin.rs | 59 ++++++++++- 7 files changed, 191 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e4e3ca3..08133bac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All significant changes to this project will be documented in this file. ### Breaking changes +* New `CountMinSketch` values use serial version 2 with XxHash64-derived row seeds and item hashes, plus unsigned bucket mapping. Deserialization still accepts legacy Rust version 1 images and preserves their version when reserializing, but version 1 images from other implementations remain incompatible for queries, updates, and merges. Merging now requires matching serial versions. * `BloomFilter::invert` is removed; use `BloomFilter::difference` for approximate A-not-B. The result excludes items in the right filter, but hash collisions can also remove items unique to the left filter. * `FrequentItemsSketch::is_empty` now returns `false` when the stream weight is nonzero, even if no items are retained. Use `num_active_items() == 0` to test for zero retained items. * `ReqSketch` queries now use `datasketches::common::SearchCriteria` instead of `datasketches::req::SearchCriteria`. `SearchCriteria` no longer implements `Default`; explicitly choose `Inclusive` or `Exclusive`. diff --git a/datasketches/src/countmin/serialization.rs b/datasketches/src/countmin/serialization.rs index 7a930e01..6ec67456 100644 --- a/datasketches/src/countmin/serialization.rs +++ b/datasketches/src/countmin/serialization.rs @@ -16,6 +16,7 @@ // under the License. pub const PREAMBLE_LONGS_SHORT: u8 = 2; -pub const SERIAL_VERSION: u8 = 1; +pub const SERIAL_VERSION_1: u8 = 1; +pub const SERIAL_VERSION_2: u8 = 2; pub const FLAGS_IS_EMPTY: u8 = 1 << 0; pub const LONG_SIZE_BYTES: usize = 8; diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 24a60e44..781b3ea1 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -21,7 +21,6 @@ use std::hash::Hasher; use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::ensure_preamble_longs_in; -use crate::codec::assert::ensure_serial_version_is; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; use crate::countmin::CountMinValue; @@ -29,11 +28,13 @@ use crate::countmin::UnsignedCountMinValue; use crate::countmin::serialization::FLAGS_IS_EMPTY; use crate::countmin::serialization::LONG_SIZE_BYTES; use crate::countmin::serialization::PREAMBLE_LONGS_SHORT; -use crate::countmin::serialization::SERIAL_VERSION; +use crate::countmin::serialization::SERIAL_VERSION_1; +use crate::countmin::serialization::SERIAL_VERSION_2; use crate::error::Error; use crate::error::ErrorKind; use crate::hash::DEFAULT_UPDATE_SEED; use crate::hash::MurmurHash3X64128; +use crate::hash::XxHash64; use crate::hash::check_seed_hash; use crate::hash::compute_seed_hash; @@ -49,6 +50,7 @@ pub struct CountMinSketch { num_buckets: u32, seed: u64, seed_hash: u16, + serial_version: u8, // Every bucket satisfies |count| <= total_weight, so a checked total also bounds bucket // additions during update/merge: |a + b| <= |a| + |b|. Deserialization validates this // invariant; unsigned halving and decay preserve it. @@ -103,6 +105,7 @@ impl CountMinSketch { num_buckets, seed, seed_hash, + SERIAL_VERSION_2, entries, )) } @@ -271,7 +274,8 @@ impl CountMinSketch { /// # Errors /// /// Returns an error without modifying the sketch if the sketches have different numbers of - /// hashes, bucket counts, or seeds, or their combined total absolute weight exceeds `T::MAX`. + /// hashes, bucket counts, seeds, or serial versions, or their combined total absolute weight + /// exceeds `T::MAX`. /// /// # Examples /// @@ -291,9 +295,10 @@ impl CountMinSketch { if self.num_hashes != other.num_hashes || self.num_buckets != other.num_buckets || self.seed != other.seed + || self.serial_version != other.serial_version { return Err(Error::invalid_argument( - "Count-Min sketches must have matching numbers of hashes, bucket counts, and seeds", + "Count-Min sketches must have matching numbers of hashes, bucket counts, seeds, and serial versions", )); } self.total_weight = self @@ -308,8 +313,12 @@ impl CountMinSketch { /// Serializes this sketch into the Count-Min binary format. /// - /// DataSketches C++ can read the serialized data, but item queries and updates there are - /// unreliable because the two implementations map items to counters differently. + /// New sketches use serial version 2. Sketches deserialized from version 1 images retain the + /// legacy Rust item-to-counter mapping and are serialized as version 1 again. + /// + /// Version 2 derives each row seed by hashing the row's 8-byte little-endian index with + /// XxHash64 and the sketch seed. It uses XxHash64 for item hashing and shifts the hash right by + /// one before taking the bucket modulo. /// /// # Examples /// @@ -333,7 +342,7 @@ impl CountMinSketch { let mut bytes = SketchBytes::with_capacity(header_size + payload_size); bytes.write_u8(PREAMBLE_LONGS_SHORT); - bytes.write_u8(SERIAL_VERSION); + bytes.write_u8(self.serial_version); bytes.write_u8(Family::COUNTMIN.id); bytes.write_u8(if self.is_empty() { FLAGS_IS_EMPTY } else { 0 }); bytes.write_u32_le(0); // unused @@ -360,9 +369,9 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the default seed. /// - /// Can read sketches serialized by DataSketches C++. - /// Item queries and updates on those sketches are unreliable because the two - /// implementations map items to counters differently. + /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; + /// queries, updates, and merges on version 1 images from other implementations remain + /// unreliable. /// /// # Errors /// @@ -386,9 +395,9 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the provided seed. /// - /// Can read sketches serialized by DataSketches C++. - /// Item queries and updates on those sketches are unreliable because the two - /// implementations map items to counters differently. + /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; + /// queries, updates, and merges on version 1 images from other implementations remain + /// unreliable. /// /// # Errors /// @@ -430,7 +439,11 @@ impl CountMinSketch { .map_err(insufficient_data(""))?; Family::COUNTMIN.validate_id(family_id)?; - ensure_serial_version_is(SERIAL_VERSION, serial_version)?; + if serial_version != SERIAL_VERSION_1 && serial_version != SERIAL_VERSION_2 { + return Err(Error::deserial(format!( + "unsupported serial version: expected {SERIAL_VERSION_1} or {SERIAL_VERSION_2}, got {serial_version}" + ))); + } ensure_preamble_longs_in(&[PREAMBLE_LONGS_SHORT], preamble_longs)?; let num_buckets = cursor @@ -468,7 +481,14 @@ impl CountMinSketch { } } - let mut sketch = Self::make(num_hashes, num_buckets, seed, expected_seed_hash, entries); + let mut sketch = Self::make( + num_hashes, + num_buckets, + seed, + expected_seed_hash, + serial_version, + entries, + ); if is_empty { return Ok(sketch); } @@ -500,14 +520,22 @@ impl CountMinSketch { + self.hash_seeds.capacity() * size_of::() } - fn make(num_hashes: u8, num_buckets: u32, seed: u64, seed_hash: u16, entries: usize) -> Self { + fn make( + num_hashes: u8, + num_buckets: u32, + seed: u64, + seed_hash: u16, + serial_version: u8, + entries: usize, + ) -> Self { let counts = vec![T::ZERO; entries]; - let hash_seeds = make_hash_seeds(seed, num_hashes); + let hash_seeds = make_hash_seeds(seed, num_hashes, serial_version); CountMinSketch { num_hashes, num_buckets, seed, seed_hash, + serial_version, total_weight: T::ZERO, counts, hash_seeds, @@ -515,10 +543,20 @@ impl CountMinSketch { } fn bucket_index(&self, item: &I, seed: u64) -> usize { - let mut hasher = MurmurHash3X64128::with_seed(seed); - item.hash(&mut hasher); - let (h1, _) = hasher.finish128(); - (h1 % self.num_buckets as u64) as usize + let hash = match self.serial_version { + SERIAL_VERSION_1 => { + let mut hasher = MurmurHash3X64128::with_seed(seed); + item.hash(&mut hasher); + hasher.finish128().0 + } + SERIAL_VERSION_2 => { + let mut hasher = XxHash64::with_seed(seed); + item.hash(&mut hasher); + hasher.finish64() >> 1 + } + _ => unreachable!("unsupported Count-Min serial version"), + }; + (hash % self.num_buckets as u64) as usize } } @@ -615,14 +653,19 @@ fn entries_for_config_checked(num_hashes: u8, num_buckets: u32) -> Result Vec { +fn make_hash_seeds(seed: u64, num_hashes: u8, serial_version: u8) -> Vec { let mut seeds = Vec::with_capacity(num_hashes as usize); for i in 0..num_hashes { - // Derive per-row hash seeds deterministically from the sketch seed. - let mut hasher = MurmurHash3X64128::with_seed(seed); - hasher.write(&u64::from(i).to_le_bytes()); - let (h1, _) = hasher.finish128(); - seeds.push(h1); + let row_seed = match serial_version { + SERIAL_VERSION_1 => { + let mut hasher = MurmurHash3X64128::with_seed(seed); + hasher.write(&u64::from(i).to_le_bytes()); + hasher.finish128().0 + } + SERIAL_VERSION_2 => XxHash64::hash_u64(u64::from(i), seed), + _ => unreachable!("unsupported Count-Min serial version"), + }; + seeds.push(row_seed); } seeds } diff --git a/datasketches/src/hash/mod.rs b/datasketches/src/hash/mod.rs index 2084902b..d0860293 100644 --- a/datasketches/src/hash/mod.rs +++ b/datasketches/src/hash/mod.rs @@ -38,9 +38,9 @@ mod murmurhash; ))] pub(crate) use self::murmurhash::*; -#[cfg(feature = "bloom")] +#[cfg(any(feature = "bloom", feature = "countmin"))] mod xxhash; -#[cfg(feature = "bloom")] +#[cfg(any(feature = "bloom", feature = "countmin"))] pub(crate) use self::xxhash::*; #[cfg(any( @@ -82,7 +82,7 @@ pub(crate) use self::seed::*; ))] pub(crate) const DEFAULT_UPDATE_SEED: u64 = 9001; -#[cfg(feature = "bloom")] +#[cfg(any(feature = "bloom", feature = "countmin"))] #[inline(always)] fn read_u32_le(bytes: &[u8]) -> u32 { u32::from_le_bytes(bytes.try_into().expect("four-byte hash input")) diff --git a/datasketches/src/hash/xxhash.rs b/datasketches/src/hash/xxhash.rs index 1ea55035..e3886a33 100644 --- a/datasketches/src/hash/xxhash.rs +++ b/datasketches/src/hash/xxhash.rs @@ -141,6 +141,16 @@ impl Hasher for XxHash64 { self.finish64() } + #[inline(always)] + fn write_i64(&mut self, value: i64) { + self.write(&value.to_le_bytes()); + } + + #[inline(always)] + fn write_u64(&mut self, value: u64) { + self.write(&value.to_le_bytes()); + } + #[inline(always)] fn write(&mut self, bytes: &[u8]) { self.total_len = self.total_len.wrapping_add(bytes.len() as u64); diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index ba2ab679..3ddbcb6a 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -400,6 +400,55 @@ fn test_serialize_deserialize_non_empty_u64() { assert_eq!(decoded.estimate(42u64), sketch.estimate(42u64)); } +#[test] +fn test_v2_known_answer_vectors() { + // Independently computed with XXH64 over little-endian `u64` values. + const NUM_BUCKETS: usize = 1000; + + for (seed, one_buckets, nine_buckets) in [ + (9001, [185, 898, 4], [490, 217, 440]), + (42, [468, 459, 696], [841, 428, 322]), + ] { + let mut sketch = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); + sketch.update_with_weight(1u64, 10); + sketch.update_with_weight(9u64, 810); + + let bytes = sketch.serialize(); + assert_eq!(bytes[1], 2); + let counts: Vec = bytes[24..] + .chunks_exact(8) + .map(|bytes| u64::from_le_bytes(bytes.try_into().unwrap())) + .collect(); + assert_eq!(counts.len(), 3 * NUM_BUCKETS); + + for row in 0..3 { + let actual: Vec<(usize, u64)> = counts[row * NUM_BUCKETS..(row + 1) * NUM_BUCKETS] + .iter() + .copied() + .enumerate() + .filter(|(_, count)| *count != 0) + .collect(); + let mut expected = vec![(one_buckets[row], 10), (nine_buckets[row], 810)]; + expected.sort_unstable_by_key(|(bucket, _)| *bucket); + assert_eq!(actual, expected, "seed {seed}, row {row}"); + } + + let mut decoded = CountMinSketch::::deserialize_with_seed(&bytes, seed).unwrap(); + assert_eq!(decoded.total_weight(), 820); + assert_eq!(decoded.estimate(1u64), 10); + assert_eq!(decoded.estimate(9u64), 810); + assert_eq!(decoded.serialize(), bytes); + + decoded.update_with_weight(1u64, 5); + assert_eq!(decoded.estimate(1u64), 15); + let mut merged = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); + merged.merge(&decoded).unwrap(); + assert_eq!(merged.total_weight(), 825); + assert_eq!(merged.estimate(1u64), 15); + assert_eq!(merged.estimate(9u64), 810); + } +} + #[test] fn test_truncated_non_empty_payload_is_rejected_before_table_allocation() { let mut bytes = CountMinSketch::::new(1, 3).unwrap().serialize(); diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index 4a62faaf..eb282c5b 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -25,6 +25,59 @@ use tests_integration::ZERO_HASH_SEED; use crate::serialization_test_data; +// Generated by CountMinSketch v0.5.0 with one row, five buckets, seed 123, and key 1 at weight 10. +const RUST_V1_SERIALIZED: [u8; 64] = [ + 2, 1, 18, 0, 0, 0, 0, 0, 5, 0, 0, 0, 1, 182, 5, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 0, 0, 0, 0, + 0, 0, 0, +]; + +#[test] +fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { + let mut sketch = + CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + assert_eq!(sketch.total_weight(), 10); + assert_eq!(sketch.estimate(1u64), 10); + assert_eq!(sketch.serialize(), RUST_V1_SERIALIZED); + + sketch.update_with_weight(1u64, 5); + assert_eq!(sketch.total_weight(), 15); + assert_eq!(sketch.estimate(1u64), 15); + assert_eq!(sketch.serialize()[1], 1); + + let mut merged = + CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + merged.merge(&sketch).unwrap(); + assert_eq!(merged.total_weight(), 25); + assert_eq!(merged.estimate(1u64), 25); +} + +#[test] +fn rejects_merge_across_serial_versions() { + let legacy = CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + let mut current = CountMinSketch::::with_seed(1, 5, 123).unwrap(); + current.update_with_weight(1u64, 10); + let before = current.clone(); + + assert_eq!( + current.merge(&legacy).unwrap_err().kind(), + ErrorKind::InvalidArgument + ); + assert_eq!(current, before); +} + +#[test] +fn rejects_unknown_serial_version() { + let mut bytes = CountMinSketch::::new(1, 5).unwrap().serialize(); + bytes[1] = 3; + let error = CountMinSketch::::deserialize(&bytes).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidData); + assert_that!( + error.message(), + contains_substring("expected 1 or 2, got 3") + ); +} + #[test] fn rejects_weights_that_cannot_bound_counter_arithmetic() { let mut sketch = CountMinSketch::::new(2, 8).unwrap(); @@ -43,9 +96,9 @@ fn rejects_weights_that_cannot_bound_counter_arithmetic() { } } -// This test validates binary format compatibility (deserialize + byte round-trip) for -// datasketches-cpp CountMin snapshots. It intentionally does not assert estimate equivalence -// against original input keys because per-row hash seed derivation differs across implementations. +// This helper validates binary format compatibility (deserialize + byte round-trip) for +// version 1 datasketches-cpp CountMin snapshots. It intentionally does not assert estimate +// equivalence because version 1 row-seed derivation differs across implementations. fn assert_cpp_snapshot( filename: &str, seed: u64, From ad973413139f2eeef61caec2f70cf410eb65cec0 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 13:10:39 +0800 Subject: [PATCH 02/13] fix(countmin): encode version 2 items portably --- CHANGELOG.md | 1 + benchmarks/countmin/estimate.rs | 5 +- benchmarks/countmin/update.rs | 3 +- datasketches/src/countmin/item.rs | 111 +++++++++ datasketches/src/countmin/mod.rs | 3 + datasketches/src/countmin/sketch.rs | 68 ++++-- datasketches/src/hash/value/mod.rs | 3 +- datasketches/src/hash/xxhash.rs | 10 - .../tests/countmin_test/items.rs | 84 +++++++ tests-integration/tests/countmin_test/main.rs | 1 + .../tests/countmin_test/sketch.rs | 59 +---- .../tests/serde_tests/countmin.rs | 228 ++++++++++++++++++ 12 files changed, 488 insertions(+), 88 deletions(-) create mode 100644 datasketches/src/countmin/item.rs create mode 100644 tests-integration/tests/countmin_test/items.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ec27be6..944fd36d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All significant changes to this project will be documented in this file. ### Breaking changes * New `CountMinSketch` values use serial version 2 with XxHash64-derived row seeds and item hashes, plus unsigned bucket mapping. Deserialization still accepts legacy Rust version 1 images and preserves their version when reserializing, but version 1 images from other implementations remain incompatible for queries, updates, and merges. Merging now requires matching serial versions. +* `CountMinSketch` updates and queries now accept `CountMinItem` instead of arbitrary `Hash` values: integers up to 64 bits, strings, byte slices, arrays, vectors, and shared references. Version 2 widens integers to 64 bits and hashes little-endian bytes, uses UTF-8 strings and raw byte contents without prefixes or terminators, and ignores empty strings and byte collections. Pass these inputs directly instead of hash wrappers; convert custom values to a supported representation and use `CountMinItem` for generic bounds. Legacy sketches populated with unsupported custom hash encodings must be rebuilt from their input data. New sketches and version 2 images require `1..=127` hash rows. ### New features diff --git a/benchmarks/countmin/estimate.rs b/benchmarks/countmin/estimate.rs index aae72067..31ddb564 100644 --- a/benchmarks/countmin/estimate.rs +++ b/benchmarks/countmin/estimate.rs @@ -20,7 +20,6 @@ use std::hint::black_box; use benchmarks::Bencher; use benchmarks::Runner; use datasketches::countmin::CountMinSketch; -use datasketches::hash::value::raw_bytes; use crate::hash_inputs::ITEMS; use crate::hash_inputs::bytes_32_values; @@ -44,13 +43,13 @@ fn bytes_32(bencher: Bencher) { let values = bytes_32_values(); let mut sketch = CountMinSketch::::new(4, 16_384).unwrap(); for value in &values { - sketch.update(raw_bytes::from_slice(value)); + sketch.update(value); } bencher.bench_local(|| { let mut total = 0u64; for value in &values { - total = total.wrapping_add(sketch.estimate(raw_bytes::from_slice(black_box(value)))); + total = total.wrapping_add(sketch.estimate(black_box(value))); } black_box(total) }); diff --git a/benchmarks/countmin/update.rs b/benchmarks/countmin/update.rs index dd6db39d..1e7cbdef 100644 --- a/benchmarks/countmin/update.rs +++ b/benchmarks/countmin/update.rs @@ -20,7 +20,6 @@ use std::hint::black_box; use benchmarks::Bencher; use benchmarks::Runner; use datasketches::countmin::CountMinSketch; -use datasketches::hash::value::raw_bytes; use crate::hash_inputs::ITEMS; use crate::hash_inputs::bytes_32_values; @@ -41,7 +40,7 @@ fn bytes_32(bencher: Bencher) { bencher.bench_local(|| { let mut sketch = CountMinSketch::::new(4, 16_384).unwrap(); for value in &values { - sketch.update(raw_bytes::from_slice(black_box(value))); + sketch.update(black_box(value)); } black_box(sketch) }); diff --git a/datasketches/src/countmin/item.rs b/datasketches/src/countmin/item.rs new file mode 100644 index 00000000..fa3bb265 --- /dev/null +++ b/datasketches/src/countmin/item.rs @@ -0,0 +1,111 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::hash::Hasher; + +use crate::hash::XxHash64; + +/// An item with a portable Count-Min encoding. +/// +/// Implemented for integers up to 64 bits (including `isize` and `usize`), `str`, `String`, +/// byte slices, byte arrays, `Vec`, and shared references to these types. This trait is sealed. +/// +/// Version 2 widens signed integers to `i64` and unsigned integers to `u64`, then hashes their +/// eight little-endian bytes. Strings use UTF-8 bytes; byte collections use their contents, +/// without a length prefix or terminator. Equal byte encodings identify the same item, even +/// across types. Empty strings and byte collections are ignored. +/// +/// Convert floating-point and custom values to one of these supported types before updating or +/// querying. Generic callers should use this bound instead of [`std::hash::Hash`]. +/// +/// Version 1 sketches retain Rust's original [`std::hash::Hash`] encoding for the supplied +/// type, including empty inputs. Use the same input type that originally populated the sketch. +pub trait CountMinItem: private::CountMinItem {} + +mod private { + use std::hash::Hash; + + pub trait CountMinItem: Hash { + fn hash_v2(&self, seed: u64) -> u64; + + fn is_empty(&self) -> bool { + false + } + } +} + +macro_rules! impl_integer { + ($($ty:ty),* $(,)?) => {$( + impl private::CountMinItem for $ty { + #[inline] + fn hash_v2(&self, seed: u64) -> u64 { + XxHash64::hash_u64(*self as u64, seed) + } + } + + impl CountMinItem for $ty {} + )*}; +} + +impl_integer!(i8, i16, i32, i64, isize, u8, u16, u32, u64, usize); + +macro_rules! impl_bytes { + ($($ty:ty),* $(,)?) => {$( + impl private::CountMinItem for $ty { + #[inline] + fn hash_v2(&self, seed: u64) -> u64 { + let mut hasher = XxHash64::with_seed(seed); + hasher.write(self.as_ref()); + hasher.finish64() + } + + fn is_empty(&self) -> bool { + self.is_empty() + } + } + + impl CountMinItem for $ty {} + )*}; +} + +impl_bytes!(str, String, [u8], Vec); + +impl private::CountMinItem for [u8; N] { + #[inline] + fn hash_v2(&self, seed: u64) -> u64 { + self.as_slice().hash_v2(seed) + } + + fn is_empty(&self) -> bool { + N == 0 + } +} + +impl CountMinItem for [u8; N] {} + +impl private::CountMinItem for &I { + #[inline] + fn hash_v2(&self, seed: u64) -> u64 { + (**self).hash_v2(seed) + } + + fn is_empty(&self) -> bool { + (**self).is_empty() + } +} + +impl CountMinItem for &I {} diff --git a/datasketches/src/countmin/mod.rs b/datasketches/src/countmin/mod.rs index d8e4f5de..f559f745 100644 --- a/datasketches/src/countmin/mod.rs +++ b/datasketches/src/countmin/mod.rs @@ -44,6 +44,9 @@ mod serialization; +mod item; +pub use self::item::CountMinItem; + mod sketch; pub use self::sketch::CountMinSketch; diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 2f0e35c9..65f452e5 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use std::hash::Hash; use std::hash::Hasher; use crate::codec::SketchBytes; @@ -23,6 +22,7 @@ use crate::codec::SketchSlice; use crate::codec::assert::ensure_preamble_longs_in; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; +use crate::countmin::CountMinItem; use crate::countmin::CountMinValue; use crate::countmin::UnsignedCountMinValue; use crate::countmin::serialization::FLAGS_IS_EMPTY; @@ -44,6 +44,7 @@ const MAX_TABLE_ENTRIES: usize = 1 << 30; /// /// The sketch provides upper and lower bounds on estimated item frequencies /// with configurable relative error and confidence. +/// Inputs follow the encoding rules of [`CountMinItem`]. #[derive(Debug, Clone, PartialEq)] pub struct CountMinSketch { num_hashes: u8, @@ -103,8 +104,8 @@ impl CountMinSketch { /// /// # Errors /// - /// Returns an error if `num_hashes` is `0`, `num_buckets` is less than `3`, or the total table - /// size exceeds the supported limit. + /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, or + /// the total table size exceeds the supported limit. /// /// # Examples /// @@ -123,7 +124,7 @@ impl CountMinSketch { /// # Errors /// /// Returns an error if any of: - /// * `num_hashes` is `0`. + /// * `num_hashes` is outside `1..=127`. /// * `num_buckets` is less than `3`. /// * The total table size exceeds the supported limit. /// * The computed seed hash is zero. @@ -222,6 +223,8 @@ impl CountMinSketch { /// Updates the sketch with a single occurrence of the item. /// + /// Version 2 ignores empty strings and byte collections. See [`CountMinItem`] for encoding. + /// /// # Panics /// /// Panics without modifying the sketch if the total absolute weight would exceed `T::MAX`. @@ -235,12 +238,15 @@ impl CountMinSketch { /// sketch.update("apple"); /// assert!(sketch.estimate("apple") >= 1); /// ``` - pub fn update(&mut self, item: I) { + pub fn update(&mut self, item: I) { self.update_with_weight(item, T::ONE); } /// Updates the sketch with the given item and weight. /// + /// Version 2 ignores empty strings and byte collections, regardless of weight. + /// See [`CountMinItem`] for encoding. + /// /// # Panics /// /// Panics without modifying the sketch if the absolute weight or the total absolute weight @@ -255,8 +261,8 @@ impl CountMinSketch { /// sketch.update_with_weight("banana", 3); /// assert!(sketch.estimate("banana") >= 3); /// ``` - pub fn update_with_weight(&mut self, item: I, weight: T) { - if weight == T::ZERO { + pub fn update_with_weight(&mut self, item: I, weight: T) { + if weight == T::ZERO || (self.serial_version == SERIAL_VERSION_2 && item.is_empty()) { return; } self.total_weight = weight @@ -273,6 +279,9 @@ impl CountMinSketch { /// Returns the estimated frequency of the given item. /// + /// Version 2 returns zero for empty strings and byte collections. + /// See [`CountMinItem`] for encoding. + /// /// # Examples /// /// ``` @@ -282,7 +291,10 @@ impl CountMinSketch { /// sketch.update_with_weight("pear", 2); /// assert!(sketch.estimate("pear") >= 2); /// ``` - pub fn estimate(&self, item: I) -> T { + pub fn estimate(&self, item: I) -> T { + if self.serial_version == SERIAL_VERSION_2 && item.is_empty() { + return T::ZERO; + } let num_buckets = self.buckets.count as usize; let mut min = T::MAX; for (row, seed) in self.hash_seeds.iter().enumerate() { @@ -297,12 +309,19 @@ impl CountMinSketch { } /// Returns the lower bound on the true frequency of the given item. - pub fn lower_bound(&self, item: I) -> T { + /// + /// Version 2 returns zero for empty strings and byte collections. + pub fn lower_bound(&self, item: I) -> T { self.estimate(item) } /// Returns the upper bound on the true frequency of the given item. - pub fn upper_bound(&self, item: I) -> T { + /// + /// Version 2 returns zero for empty strings and byte collections. + pub fn upper_bound(&self, item: I) -> T { + if self.serial_version == SERIAL_VERSION_2 && item.is_empty() { + return T::ZERO; + } let estimate = self.estimate(item); let error = self.total_weight.scale_nonnegative(self.relative_error()); estimate.checked_add(error).unwrap_or(T::MAX) @@ -355,9 +374,9 @@ impl CountMinSketch { /// New sketches use serial version 2. Sketches deserialized from version 1 images retain the /// legacy Rust item-to-counter mapping and are serialized as version 1 again. /// - /// Version 2 derives each row seed by hashing the row's 8-byte little-endian index with - /// XxHash64 and the sketch seed. It uses XxHash64 for item hashing and shifts the hash right by - /// one before taking the bucket modulo. + /// Version 2 uses the portable DataSketches Count-Min format and [`CountMinItem`] encoding. + /// Use `i64` counters for cross-language interchange; other counter types are compatible only + /// when their eight-byte values can be interpreted as signed 64-bit integers by the reader. /// /// # Examples /// @@ -408,6 +427,9 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the default seed. /// + /// Reads portable version 2 images using [`CountMinItem`] encoding. Counter values must fit + /// `T`. + /// /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; /// queries, updates, and merges on version 1 images from other implementations remain /// unreliable. @@ -434,6 +456,9 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the provided seed. /// + /// Reads portable version 2 images using [`CountMinItem`] encoding. Counter values must fit + /// `T`. + /// /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; /// queries, updates, and merges on version 1 images from other implementations remain /// unreliable. @@ -489,6 +514,11 @@ impl CountMinSketch { .read_u32_le() .map_err(insufficient_data("num_buckets"))?; let num_hashes = cursor.read_u8().map_err(insufficient_data("num_hashes"))?; + if serial_version == SERIAL_VERSION_2 && num_hashes > 127 { + return Err(Error::deserial( + "version 2 num_hashes must be within 1..=127", + )); + } let seed_hash = cursor .read_u16_le() .map_err(insufficient_data("seed_hash"))?; @@ -579,18 +609,14 @@ impl CountMinSketch { } } - fn bucket_index(&self, item: &I, seed: u64) -> usize { + fn bucket_index(&self, item: &I, seed: u64) -> usize { let hash = match self.serial_version { SERIAL_VERSION_1 => { let mut hasher = MurmurHash3X64128::with_seed(seed); item.hash(&mut hasher); hasher.finish128().0 } - SERIAL_VERSION_2 => { - let mut hasher = XxHash64::with_seed(seed); - item.hash(&mut hasher); - hasher.finish64() >> 1 - } + SERIAL_VERSION_2 => item.hash_v2(seed) >> 1, _ => unreachable!("unsupported Count-Min serial version"), }; self.buckets.index(hash) @@ -655,8 +681,8 @@ impl CountMinSketch { } fn entries_for_config(num_hashes: u8, num_buckets: u32) -> Result { - if num_hashes == 0 { - return Err(Error::invalid_argument("num_hashes must be at least 1")); + if !(1..=127).contains(&num_hashes) { + return Err(Error::invalid_argument("num_hashes must be within 1..=127")); } if num_buckets < 3 { return Err(Error::invalid_argument("num_buckets must be at least 3")); diff --git a/datasketches/src/hash/value/mod.rs b/datasketches/src/hash/value/mod.rs index a4867a3d..175412bf 100644 --- a/datasketches/src/hash/value/mod.rs +++ b/datasketches/src/hash/value/mod.rs @@ -17,9 +17,10 @@ //! Hashable value wrappers for sketches. //! -//! Sketch update APIs accept any value that implements [`Hash`]. For most Rust values, +//! Most sketch update APIs accept any value that implements [`Hash`]. For most Rust values, //! passing the value directly is sufficient. This module provides value wrappers for //! cases where the default implementation does not match a sketch's compatibility rules. +//! Count-Min instead accepts its own set of integer, string, and byte inputs directly. //! //! ## Floating-point Numbers //! diff --git a/datasketches/src/hash/xxhash.rs b/datasketches/src/hash/xxhash.rs index e3886a33..1ea55035 100644 --- a/datasketches/src/hash/xxhash.rs +++ b/datasketches/src/hash/xxhash.rs @@ -141,16 +141,6 @@ impl Hasher for XxHash64 { self.finish64() } - #[inline(always)] - fn write_i64(&mut self, value: i64) { - self.write(&value.to_le_bytes()); - } - - #[inline(always)] - fn write_u64(&mut self, value: u64) { - self.write(&value.to_le_bytes()); - } - #[inline(always)] fn write(&mut self, bytes: &[u8]) { self.total_len = self.total_len.wrapping_add(bytes.len() as u64); diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs new file mode 100644 index 00000000..b57c6b80 --- /dev/null +++ b/tests-integration/tests/countmin_test/items.rs @@ -0,0 +1,84 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use datasketches::countmin::CountMinItem; +use datasketches::countmin::CountMinSketch; + +fn assert_same_item(left: I, right: J) { + let mut a = CountMinSketch::::new(3, 1000).unwrap(); + let mut b = a.clone(); + a.update_with_weight(&left, 7); + b.update_with_weight(&right, 7); + assert_eq!(a, b); + assert_eq!(a.estimate(right), 7); + assert_eq!(b.estimate(left), 7); +} + +#[test] +fn integer_widths_use_the_same_64_bit_encoding() { + assert_same_item(-1i8, -1i64); + assert_same_item(-1i16, -1i64); + assert_same_item(-1i32, -1i64); + assert_same_item(-1isize, -1i64); + assert_same_item(u8::MAX, 255u64); + assert_same_item(u16::MAX, 65535u64); + assert_same_item(u32::MAX, u32::MAX as u64); + assert_same_item(1usize, 1u64); + assert_same_item(1i32, 1u64); + assert_same_item(-1i64, u64::MAX); + assert_same_item(i64::MIN, 1u64 << 63); + assert_same_item(0x0102_0304_0506_0708u64, [8u8, 7, 6, 5, 4, 3, 2, 1]); +} + +#[test] +fn owned_and_borrowed_strings_and_bytes_identify_the_same_item() { + let text = "été and a long UTF-8 string crossing the 32-byte hash block boundary"; + assert_same_item(text, text.to_owned()); + assert_same_item(text, text.as_bytes()); + assert_same_item(text, text.as_bytes().to_vec()); + assert_same_item("apple", *b"apple"); + assert_same_item("apple", b"apple"); + assert_same_item("été", [0xc3u8, 0xa9, 0x74, 0xc3, 0xa9]); +} + +#[test] +fn empty_items_are_ignored_even_in_a_full_sketch() { + fn check(sketch: &mut CountMinSketch, item: I) { + let before = sketch.clone(); + sketch.update(&item); + sketch.update_with_weight(&item, i64::MIN); + assert_eq!(*sketch, before); + assert_eq!(sketch.estimate(&item), 0); + assert_eq!(sketch.lower_bound(&item), 0); + assert_eq!(sketch.upper_bound(item), 0); + } + + for filled in [false, true] { + let mut sketch = CountMinSketch::::new(1, 3).unwrap(); + if filled { + for i in 0..100 { + sketch.update(i); + } + } + check(&mut sketch, ""); + check(&mut sketch, String::new()); + check(&mut sketch, b"".as_slice()); + check(&mut sketch, [] as [u8; 0]); + check(&mut sketch, b""); + check(&mut sketch, Vec::::new()); + } +} diff --git a/tests-integration/tests/countmin_test/main.rs b/tests-integration/tests/countmin_test/main.rs index 825a6281..bf51e28c 100644 --- a/tests-integration/tests/countmin_test/main.rs +++ b/tests-integration/tests/countmin_test/main.rs @@ -15,4 +15,5 @@ // specific language governing permissions and limitations // under the License. +mod items; mod sketch; diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index 24b6b1be..8710c1cf 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -400,55 +400,6 @@ fn test_serialize_deserialize_non_empty_u64() { assert_eq!(decoded.estimate(42u64), sketch.estimate(42u64)); } -#[test] -fn test_v2_known_answer_vectors() { - // Independently computed with XXH64 over little-endian `u64` values. - const NUM_BUCKETS: usize = 1000; - - for (seed, one_buckets, nine_buckets) in [ - (9001, [185, 898, 4], [490, 217, 440]), - (42, [468, 459, 696], [841, 428, 322]), - ] { - let mut sketch = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); - sketch.update_with_weight(1u64, 10); - sketch.update_with_weight(9u64, 810); - - let bytes = sketch.serialize(); - assert_eq!(bytes[1], 2); - let counts: Vec = bytes[24..] - .chunks_exact(8) - .map(|bytes| u64::from_le_bytes(bytes.try_into().unwrap())) - .collect(); - assert_eq!(counts.len(), 3 * NUM_BUCKETS); - - for row in 0..3 { - let actual: Vec<(usize, u64)> = counts[row * NUM_BUCKETS..(row + 1) * NUM_BUCKETS] - .iter() - .copied() - .enumerate() - .filter(|(_, count)| *count != 0) - .collect(); - let mut expected = vec![(one_buckets[row], 10), (nine_buckets[row], 810)]; - expected.sort_unstable_by_key(|(bucket, _)| *bucket); - assert_eq!(actual, expected, "seed {seed}, row {row}"); - } - - let mut decoded = CountMinSketch::::deserialize_with_seed(&bytes, seed).unwrap(); - assert_eq!(decoded.total_weight(), 820); - assert_eq!(decoded.estimate(1u64), 10); - assert_eq!(decoded.estimate(9u64), 810); - assert_eq!(decoded.serialize(), bytes); - - decoded.update_with_weight(1u64, 5); - assert_eq!(decoded.estimate(1u64), 15); - let mut merged = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); - merged.merge(&decoded).unwrap(); - assert_eq!(merged.total_weight(), 825); - assert_eq!(merged.estimate(1u64), 15); - assert_eq!(merged.estimate(9u64), 810); - } -} - #[test] fn test_truncated_non_empty_payload_is_rejected_before_table_allocation() { let mut bytes = CountMinSketch::::new(1, 3).unwrap().serialize(); @@ -464,8 +415,14 @@ fn test_truncated_non_empty_payload_is_rejected_before_table_allocation() { #[test] fn test_invalid_hashes_return_error() { - let error = CountMinSketch::::new(0, 5).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); + for rows in [0, 128, 255] { + let error = CountMinSketch::::new(rows, 5).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + assert_eq!( + CountMinSketch::::new(127, 3).unwrap().num_hashes(), + 127 + ); } #[test] diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index eb282c5b..1cd65e51 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -17,6 +17,7 @@ use std::fs; +use datasketches::countmin::CountMinItem; use datasketches::countmin::CountMinSketch; use datasketches::error::ErrorKind; use googletest::assert_that; @@ -25,6 +26,184 @@ use tests_integration::ZERO_HASH_SEED; use crate::serialization_test_data; +#[test] +fn v2_reads_and_writes_the_published_image() { + // TCK discussion #18, section 7.4: update(1, 10), update(9, 810), update("apple", 5). + let words: [u64; 21] = [ + 0x0000_0000_0012_0202, + 0x0093_cc02_0000_0009, + 825, + 810, + 0, + 0, + 0, + 5, + 0, + 10, + 0, + 0, + 0, + 5, + 810, + 0, + 0, + 0, + 0, + 10, + 0, + ]; + let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); + let mut sketch = CountMinSketch::::new(2, 9).unwrap(); + let mut empty = bytes[..16].to_vec(); + empty[3] = 1; + assert_eq!(sketch.serialize(), empty); + assert_eq!(CountMinSketch::::deserialize(&empty).unwrap(), sketch); + + sketch.update_with_weight(1, 10); + sketch.update_with_weight(9, 810); + sketch.update_with_weight("apple", 5); + assert_eq!(sketch.serialize(), bytes); + + let mut restored = CountMinSketch::::deserialize(&bytes).unwrap(); + assert_eq!(restored, sketch); + assert_eq!(restored.estimate(1u64), 10); + assert_eq!(restored.estimate(9i8), 810); + assert_eq!(restored.estimate(b"apple"), 5); + restored.update_with_weight(b"apple".as_slice(), -2); + assert_eq!(restored.estimate("apple"), 3); + assert_eq!(restored.total_weight(), 827); + sketch.merge(&restored).unwrap(); + assert_eq!(sketch.estimate("apple"), 8); + assert_eq!(sketch.total_weight(), 1652); +} + +#[test] +fn v2_matches_published_string_and_signed_integer_buckets() { + fn check(item: I, expected: [usize; 3]) { + let mut sketch = CountMinSketch::::new(3, 1000).unwrap(); + sketch.update_with_weight(&item, 7); + let bytes = sketch.serialize(); + for (row, bucket) in expected.into_iter().enumerate() { + let row_start = 24 + row * 1000 * 8; + let counts: Vec<(usize, i64)> = bytes[row_start..row_start + 1000 * 8] + .chunks_exact(8) + .map(|bytes| i64::from_le_bytes(bytes.try_into().unwrap())) + .enumerate() + .filter(|(_, count)| *count != 0) + .collect(); + assert_eq!(counts, [(bucket, 7)]); + } + let restored = CountMinSketch::::deserialize(&bytes).unwrap(); + assert_eq!(restored.estimate(item), 7); + } + + // Values computed with DataSketches Java XxHash64 in TCK discussion #18, section 7.3. + check(-1i64, [256, 641, 308]); + check(i64::MIN, [951, 0, 298]); + check("apple", [130, 747, 144]); + check("été", [806, 491, 163]); + check(b"apple".as_slice(), [130, 747, 144]); + check("été".as_bytes(), [806, 491, 163]); +} + +#[test] +fn legacy_strings_bytes_and_narrow_integers_keep_their_hash_encoding() { + // Generated by Rust v0.6.0: three rows, 16 buckets, default seed. Updates: + // ("apple", 5), (&b"apple"[..], 7), (-1i8, 11), ("", 13), (&b""[..], 17). + let words: [u64; 51] = [ + 1179906, + 41601134833434640, + 53, + 0, + 0, + 0, + 17, + 24, + 0, + 0, + 5, + 0, + 0, + 0, + 7, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 7, + 5, + 17, + 24, + 0, + 0, + 0, + 0, + 0, + 17, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 29, + 0, + 7, + 0, + 0, + 0, + 0, + 0, + ]; + let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); + let mut sketch = CountMinSketch::::deserialize(&bytes).unwrap(); + assert_eq!(sketch.serialize(), bytes); + assert_eq!(sketch.estimate("apple"), 5); + assert_eq!(sketch.estimate(b"apple".as_slice()), 7); + assert_eq!(sketch.estimate(-1i8), 24); + assert_eq!(sketch.estimate(""), 24); + assert_eq!(sketch.estimate(b"".as_slice()), 17); + + sketch.update_with_weight("apple", 2); + sketch.update_with_weight(b"apple".as_slice(), 3); + sketch.update_with_weight("", 4); + sketch.update_with_weight(b"".as_slice(), 5); + assert_eq!(sketch.estimate("apple"), 7); + assert_eq!(sketch.estimate(b"apple".as_slice()), 10); + assert_eq!(sketch.estimate(""), 28); + assert_eq!(sketch.estimate(b"".as_slice()), 22); + assert_eq!(sketch.total_weight(), 67); + assert_eq!(sketch.serialize()[1], 1); + assert_eq!( + CountMinSketch::::deserialize(&sketch.serialize()).unwrap(), + sketch + ); +} + +#[test] +fn v2_row_limit_does_not_reject_legacy_images() { + let mut bytes = CountMinSketch::::new(1, 3).unwrap().serialize(); + bytes[12] = 128; + assert_eq!( + CountMinSketch::::deserialize(&bytes) + .unwrap_err() + .kind(), + ErrorKind::InvalidData + ); + bytes[1] = 1; + let legacy = CountMinSketch::::deserialize(&bytes).unwrap(); + assert_eq!(legacy.num_hashes(), 128); + assert_eq!(legacy.serialize(), bytes); +} + // Generated by CountMinSketch v0.5.0 with one row, five buckets, seed 123, and key 1 at weight 10. const RUST_V1_SERIALIZED: [u8; 64] = [ 2, 1, 18, 0, 0, 0, 0, 0, 5, 0, 0, 0, 1, 182, 5, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -151,3 +330,52 @@ fn zero_seed_hash_uses_the_callers_error_kind() { CountMinSketch::::deserialize_with_seed(&bytes, ZERO_HASH_SEED).unwrap_err(); assert_eq!(deserialization_error.kind(), ErrorKind::InvalidData); } + +#[test] +fn test_v2_known_answer_vectors() { + // Independently computed with XXH64 over little-endian `u64` values. + const NUM_BUCKETS: usize = 1000; + + for (seed, one_buckets, nine_buckets) in [ + (9001, [185, 898, 4], [490, 217, 440]), + (42, [468, 459, 696], [841, 428, 322]), + ] { + let mut sketch = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); + sketch.update_with_weight(1u64, 10); + sketch.update_with_weight(9u64, 810); + + let bytes = sketch.serialize(); + assert_eq!(bytes[1], 2); + let counts: Vec = bytes[24..] + .chunks_exact(8) + .map(|bytes| u64::from_le_bytes(bytes.try_into().unwrap())) + .collect(); + assert_eq!(counts.len(), 3 * NUM_BUCKETS); + + for row in 0..3 { + let actual: Vec<(usize, u64)> = counts[row * NUM_BUCKETS..(row + 1) * NUM_BUCKETS] + .iter() + .copied() + .enumerate() + .filter(|(_, count)| *count != 0) + .collect(); + let mut expected = vec![(one_buckets[row], 10), (nine_buckets[row], 810)]; + expected.sort_unstable_by_key(|(bucket, _)| *bucket); + assert_eq!(actual, expected, "seed {seed}, row {row}"); + } + + let mut decoded = CountMinSketch::::deserialize_with_seed(&bytes, seed).unwrap(); + assert_eq!(decoded.total_weight(), 820); + assert_eq!(decoded.estimate(1u64), 10); + assert_eq!(decoded.estimate(9u64), 810); + assert_eq!(decoded.serialize(), bytes); + + decoded.update_with_weight(1u64, 5); + assert_eq!(decoded.estimate(1u64), 15); + let mut merged = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); + merged.merge(&decoded).unwrap(); + assert_eq!(merged.total_weight(), 825); + assert_eq!(merged.estimate(1u64), 15); + assert_eq!(merged.estimate(9u64), 810); + } +} From f3e284cfd51e932a7a219ca6342a50b8ef4c7a78 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 13:51:14 +0800 Subject: [PATCH 03/13] fix(countmin): preserve legacy hashing APIs and make v2 opt-in --- CHANGELOG.md | 7 +- benchmarks/countmin/estimate.rs | 5 +- benchmarks/countmin/update.rs | 3 +- datasketches/src/countmin/item.rs | 111 --------- datasketches/src/countmin/mod.rs | 3 - datasketches/src/countmin/sketch.rs | 233 ++++++++++++++---- datasketches/src/hash/value/mod.rs | 3 +- .../tests/countmin_test/items.rs | 117 +++++---- .../tests/countmin_test/sketch.rs | 10 +- .../tests/serde_tests/countmin.rs | 108 ++++++-- 10 files changed, 353 insertions(+), 247 deletions(-) delete mode 100644 datasketches/src/countmin/item.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 944fd36d..2fa9f86d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,13 +4,10 @@ All significant changes to this project will be documented in this file. ## Unreleased -### Breaking changes - -* New `CountMinSketch` values use serial version 2 with XxHash64-derived row seeds and item hashes, plus unsigned bucket mapping. Deserialization still accepts legacy Rust version 1 images and preserves their version when reserializing, but version 1 images from other implementations remain incompatible for queries, updates, and merges. Merging now requires matching serial versions. -* `CountMinSketch` updates and queries now accept `CountMinItem` instead of arbitrary `Hash` values: integers up to 64 bits, strings, byte slices, arrays, vectors, and shared references. Version 2 widens integers to 64 bits and hashes little-endian bytes, uses UTF-8 strings and raw byte contents without prefixes or terminators, and ignores empty strings and byte collections. Pass these inputs directly instead of hash wrappers; convert custom values to a supported representation and use `CountMinItem` for generic bounds. Legacy sketches populated with unsupported custom hash encodings must be rebuilt from their input data. New sketches and version 2 images require `1..=127` hash rows. - ### New features +* Add opt-in Count-Min serial version 2 through `CountMinSketch::new_v2` and `with_seed_v2`, with portable row seeds, item hashing, and bucket mapping. Default constructors and version 1 images retain their existing hashing and serialization behavior, including custom `Hash` inputs and value wrappers. Version 2 interchange requires the documented input encodings; sketches of different versions cannot be merged. +* Add raw-byte update and query methods to `CountMinSketch`, with UTF-8 string support through `as_bytes()` and automatic empty-input handling. * Add `HllUnion::serialize` to serialize union results directly. * Add `XorFilter` and `XorFilterBuilder` behind the `xor` feature for immutable membership queries on values or precomputed hashes. Filters support 8-, 16-, and 32-bit fingerprints and can be serialized for later queries. diff --git a/benchmarks/countmin/estimate.rs b/benchmarks/countmin/estimate.rs index 31ddb564..aae72067 100644 --- a/benchmarks/countmin/estimate.rs +++ b/benchmarks/countmin/estimate.rs @@ -20,6 +20,7 @@ use std::hint::black_box; use benchmarks::Bencher; use benchmarks::Runner; use datasketches::countmin::CountMinSketch; +use datasketches::hash::value::raw_bytes; use crate::hash_inputs::ITEMS; use crate::hash_inputs::bytes_32_values; @@ -43,13 +44,13 @@ fn bytes_32(bencher: Bencher) { let values = bytes_32_values(); let mut sketch = CountMinSketch::::new(4, 16_384).unwrap(); for value in &values { - sketch.update(value); + sketch.update(raw_bytes::from_slice(value)); } bencher.bench_local(|| { let mut total = 0u64; for value in &values { - total = total.wrapping_add(sketch.estimate(black_box(value))); + total = total.wrapping_add(sketch.estimate(raw_bytes::from_slice(black_box(value)))); } black_box(total) }); diff --git a/benchmarks/countmin/update.rs b/benchmarks/countmin/update.rs index 1e7cbdef..dd6db39d 100644 --- a/benchmarks/countmin/update.rs +++ b/benchmarks/countmin/update.rs @@ -20,6 +20,7 @@ use std::hint::black_box; use benchmarks::Bencher; use benchmarks::Runner; use datasketches::countmin::CountMinSketch; +use datasketches::hash::value::raw_bytes; use crate::hash_inputs::ITEMS; use crate::hash_inputs::bytes_32_values; @@ -40,7 +41,7 @@ fn bytes_32(bencher: Bencher) { bencher.bench_local(|| { let mut sketch = CountMinSketch::::new(4, 16_384).unwrap(); for value in &values { - sketch.update(black_box(value)); + sketch.update(raw_bytes::from_slice(black_box(value))); } black_box(sketch) }); diff --git a/datasketches/src/countmin/item.rs b/datasketches/src/countmin/item.rs deleted file mode 100644 index fa3bb265..00000000 --- a/datasketches/src/countmin/item.rs +++ /dev/null @@ -1,111 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use std::hash::Hasher; - -use crate::hash::XxHash64; - -/// An item with a portable Count-Min encoding. -/// -/// Implemented for integers up to 64 bits (including `isize` and `usize`), `str`, `String`, -/// byte slices, byte arrays, `Vec`, and shared references to these types. This trait is sealed. -/// -/// Version 2 widens signed integers to `i64` and unsigned integers to `u64`, then hashes their -/// eight little-endian bytes. Strings use UTF-8 bytes; byte collections use their contents, -/// without a length prefix or terminator. Equal byte encodings identify the same item, even -/// across types. Empty strings and byte collections are ignored. -/// -/// Convert floating-point and custom values to one of these supported types before updating or -/// querying. Generic callers should use this bound instead of [`std::hash::Hash`]. -/// -/// Version 1 sketches retain Rust's original [`std::hash::Hash`] encoding for the supplied -/// type, including empty inputs. Use the same input type that originally populated the sketch. -pub trait CountMinItem: private::CountMinItem {} - -mod private { - use std::hash::Hash; - - pub trait CountMinItem: Hash { - fn hash_v2(&self, seed: u64) -> u64; - - fn is_empty(&self) -> bool { - false - } - } -} - -macro_rules! impl_integer { - ($($ty:ty),* $(,)?) => {$( - impl private::CountMinItem for $ty { - #[inline] - fn hash_v2(&self, seed: u64) -> u64 { - XxHash64::hash_u64(*self as u64, seed) - } - } - - impl CountMinItem for $ty {} - )*}; -} - -impl_integer!(i8, i16, i32, i64, isize, u8, u16, u32, u64, usize); - -macro_rules! impl_bytes { - ($($ty:ty),* $(,)?) => {$( - impl private::CountMinItem for $ty { - #[inline] - fn hash_v2(&self, seed: u64) -> u64 { - let mut hasher = XxHash64::with_seed(seed); - hasher.write(self.as_ref()); - hasher.finish64() - } - - fn is_empty(&self) -> bool { - self.is_empty() - } - } - - impl CountMinItem for $ty {} - )*}; -} - -impl_bytes!(str, String, [u8], Vec); - -impl private::CountMinItem for [u8; N] { - #[inline] - fn hash_v2(&self, seed: u64) -> u64 { - self.as_slice().hash_v2(seed) - } - - fn is_empty(&self) -> bool { - N == 0 - } -} - -impl CountMinItem for [u8; N] {} - -impl private::CountMinItem for &I { - #[inline] - fn hash_v2(&self, seed: u64) -> u64 { - (**self).hash_v2(seed) - } - - fn is_empty(&self) -> bool { - (**self).is_empty() - } -} - -impl CountMinItem for &I {} diff --git a/datasketches/src/countmin/mod.rs b/datasketches/src/countmin/mod.rs index f559f745..d8e4f5de 100644 --- a/datasketches/src/countmin/mod.rs +++ b/datasketches/src/countmin/mod.rs @@ -44,9 +44,6 @@ mod serialization; -mod item; -pub use self::item::CountMinItem; - mod sketch; pub use self::sketch::CountMinSketch; diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 65f452e5..b77e9561 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use std::hash::Hash; use std::hash::Hasher; use crate::codec::SketchBytes; @@ -22,7 +23,6 @@ use crate::codec::SketchSlice; use crate::codec::assert::ensure_preamble_longs_in; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; -use crate::countmin::CountMinItem; use crate::countmin::CountMinValue; use crate::countmin::UnsignedCountMinValue; use crate::countmin::serialization::FLAGS_IS_EMPTY; @@ -37,14 +37,51 @@ use crate::hash::MurmurHash3X64128; use crate::hash::XxHash64; use crate::hash::check_seed_hash; use crate::hash::compute_seed_hash; +use crate::hash::value::raw_bytes; const MAX_TABLE_ENTRIES: usize = 1 << 30; +// V2 specifies little-endian integer inputs. Keep this policy local to Count-Min so other +// sketches and the V1 path retain their existing Hash encodings. +struct V2Hasher(XxHash64); + +impl Hasher for V2Hasher { + fn finish(&self) -> u64 { + self.0.finish64() + } + + fn write(&mut self, bytes: &[u8]) { + self.0.write(bytes); + } + + fn write_i64(&mut self, value: i64) { + self.write(&value.to_le_bytes()); + } + + fn write_u64(&mut self, value: u64) { + self.write(&value.to_le_bytes()); + } +} + /// CountMin sketch for estimating item frequencies. /// /// The sketch provides upper and lower bounds on estimated item frequencies /// with configurable relative error and confidence. -/// Inputs follow the encoding rules of [`CountMinItem`]. +/// [`Self::new`] and [`Self::with_seed`] create version 1 sketches, retaining Rust's existing +/// hashing behavior. [`Self::new_v2`] and [`Self::with_seed_v2`] opt into portable version 2 +/// hashing. Versions cannot be merged with each other. +/// +/// For version 2 cross-language interchange, use `i64` counters and encode items as follows: +/// * `i64` and `u64` are hashed as eight little-endian bytes. Widen smaller integers first, or use +/// [`crate::hash::value::natural_extend`]. +/// * Strings use UTF-8 and byte arrays use their contents without prefixes or terminators. Use +/// [`Self::update_bytes`] and [`Self::estimate_bytes`], or [`raw_bytes`] wrappers. +/// * The byte methods ignore empty inputs. With wrappers, skip empty inputs before updating and +/// return zero for empty queries. +/// +/// Generic methods retain the supplied [`Hash`] encoding, including for empty inputs. Bare +/// strings, slices, and arbitrary custom `Hash` implementations are not automatically portable. +/// Use the same encoding for updates and queries; version 1 data must keep its original encoding. #[derive(Debug, Clone, PartialEq)] pub struct CountMinSketch { num_hashes: u8, @@ -100,11 +137,11 @@ impl BucketCount { } impl CountMinSketch { - /// Creates a new CountMin sketch with the default seed. + /// Creates a version 1 CountMin sketch with the default seed. /// /// # Errors /// - /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, or + /// Returns an error if `num_hashes` is `0`, `num_buckets` is less than `3`, or /// the total table size exceeds the supported limit. /// /// # Examples @@ -119,12 +156,12 @@ impl CountMinSketch { Self::with_seed(num_hashes, num_buckets, DEFAULT_UPDATE_SEED) } - /// Creates a new CountMin sketch with the provided seed. + /// Creates a version 1 CountMin sketch with the provided seed. /// /// # Errors /// /// Returns an error if any of: - /// * `num_hashes` is outside `1..=127`. + /// * `num_hashes` is `0`. /// * `num_buckets` is less than `3`. /// * The total table size exceeds the supported limit. /// * The computed seed hash is zero. @@ -138,16 +175,49 @@ impl CountMinSketch { /// assert_eq!(sketch.seed(), 42); /// ``` pub fn with_seed(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { - let entries = entries_for_config(num_hashes, num_buckets)?; - let seed_hash = compute_seed_hash(seed, ErrorKind::InvalidArgument)?; - Ok(Self::make( - num_hashes, - num_buckets, - seed, - seed_hash, - SERIAL_VERSION_2, - entries, - )) + Self::create(num_hashes, num_buckets, seed, SERIAL_VERSION_1) + } + + /// Creates a version 2 CountMin sketch with the default seed. + /// + /// See [`CountMinSketch`] for portable input encodings. Version 2 sketches cannot be merged + /// with version 1 sketches or read by libraries that support only version 1. + /// + /// # Errors + /// + /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, or + /// the total table size exceeds the supported limit. + /// + /// # Examples + /// + /// ``` + /// use datasketches::countmin::CountMinSketch; + /// use datasketches::hash::value::raw_bytes; + /// + /// let mut sketch = CountMinSketch::::new_v2(3, 1000)?; + /// sketch.update_bytes("apple".as_bytes()); + /// assert_eq!(sketch.estimate(raw_bytes::from_str("apple")), 1); + /// # Ok::<(), datasketches::error::Error>(()) + /// ``` + pub fn new_v2(num_hashes: u8, num_buckets: u32) -> Result { + Self::with_seed_v2(num_hashes, num_buckets, DEFAULT_UPDATE_SEED) + } + + /// Creates a version 2 CountMin sketch with the provided seed. + /// + /// See [`CountMinSketch`] for portable input encodings. + /// + /// # Errors + /// + /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, the + /// total table size exceeds the supported limit, or the computed seed hash is zero. + pub fn with_seed_v2(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { + if num_hashes > 127 { + return Err(Error::invalid_argument( + "version 2 num_hashes must be within 1..=127", + )); + } + Self::create(num_hashes, num_buckets, seed, SERIAL_VERSION_2) } /// Returns the number of hash functions used by the sketch. @@ -223,8 +293,6 @@ impl CountMinSketch { /// Updates the sketch with a single occurrence of the item. /// - /// Version 2 ignores empty strings and byte collections. See [`CountMinItem`] for encoding. - /// /// # Panics /// /// Panics without modifying the sketch if the total absolute weight would exceed `T::MAX`. @@ -238,15 +306,12 @@ impl CountMinSketch { /// sketch.update("apple"); /// assert!(sketch.estimate("apple") >= 1); /// ``` - pub fn update(&mut self, item: I) { + pub fn update(&mut self, item: I) { self.update_with_weight(item, T::ONE); } /// Updates the sketch with the given item and weight. /// - /// Version 2 ignores empty strings and byte collections, regardless of weight. - /// See [`CountMinItem`] for encoding. - /// /// # Panics /// /// Panics without modifying the sketch if the absolute weight or the total absolute weight @@ -261,8 +326,8 @@ impl CountMinSketch { /// sketch.update_with_weight("banana", 3); /// assert!(sketch.estimate("banana") >= 3); /// ``` - pub fn update_with_weight(&mut self, item: I, weight: T) { - if weight == T::ZERO || (self.serial_version == SERIAL_VERSION_2 && item.is_empty()) { + pub fn update_with_weight(&mut self, item: I, weight: T) { + if weight == T::ZERO { return; } self.total_weight = weight @@ -279,9 +344,6 @@ impl CountMinSketch { /// Returns the estimated frequency of the given item. /// - /// Version 2 returns zero for empty strings and byte collections. - /// See [`CountMinItem`] for encoding. - /// /// # Examples /// /// ``` @@ -291,10 +353,7 @@ impl CountMinSketch { /// sketch.update_with_weight("pear", 2); /// assert!(sketch.estimate("pear") >= 2); /// ``` - pub fn estimate(&self, item: I) -> T { - if self.serial_version == SERIAL_VERSION_2 && item.is_empty() { - return T::ZERO; - } + pub fn estimate(&self, item: I) -> T { let num_buckets = self.buckets.count as usize; let mut min = T::MAX; for (row, seed) in self.hash_seeds.iter().enumerate() { @@ -309,24 +368,73 @@ impl CountMinSketch { } /// Returns the lower bound on the true frequency of the given item. - /// - /// Version 2 returns zero for empty strings and byte collections. - pub fn lower_bound(&self, item: I) -> T { + pub fn lower_bound(&self, item: I) -> T { self.estimate(item) } /// Returns the upper bound on the true frequency of the given item. - /// - /// Version 2 returns zero for empty strings and byte collections. - pub fn upper_bound(&self, item: I) -> T { - if self.serial_version == SERIAL_VERSION_2 && item.is_empty() { - return T::ZERO; - } + pub fn upper_bound(&self, item: I) -> T { let estimate = self.estimate(item); let error = self.total_weight.scale_nonnegative(self.relative_error()); estimate.checked_add(error).unwrap_or(T::MAX) } + /// Updates the sketch with one occurrence of raw bytes, ignoring an empty input. + /// + /// Equivalent to updating with [`raw_bytes::from_slice`] for non-empty inputs. Strings can + /// be passed as UTF-8 with `str::as_bytes`. Uses the sketch's current hash version. + /// + /// # Panics + /// + /// Panics without modifying the sketch if the total absolute weight would exceed `T::MAX`. + pub fn update_bytes(&mut self, item: &[u8]) { + self.update_bytes_with_weight(item, T::ONE); + } + + /// Updates the sketch with raw bytes and a weight, ignoring an empty input regardless of + /// weight. + /// + /// Uses the same encoding as [`Self::update_bytes`]. + /// + /// # Panics + /// + /// Panics without modifying the sketch if the absolute weight or the total absolute weight + /// cannot be represented by `T`. + pub fn update_bytes_with_weight(&mut self, item: &[u8], weight: T) { + if !item.is_empty() { + self.update_with_weight(raw_bytes::from_slice(item), weight); + } + } + + /// Returns the estimated frequency of raw bytes, or zero for an empty input. + /// + /// Uses the same encoding as [`Self::update_bytes`]. + pub fn estimate_bytes(&self, item: &[u8]) -> T { + if item.is_empty() { + T::ZERO + } else { + self.estimate(raw_bytes::from_slice(item)) + } + } + + /// Returns the lower frequency bound for raw bytes, or zero for an empty input. + /// + /// Uses the same encoding as [`Self::update_bytes`]. + pub fn lower_bound_bytes(&self, item: &[u8]) -> T { + self.estimate_bytes(item) + } + + /// Returns the upper frequency bound for raw bytes, or zero for an empty input. + /// + /// Uses the same encoding as [`Self::update_bytes`]. + pub fn upper_bound_bytes(&self, item: &[u8]) -> T { + if item.is_empty() { + T::ZERO + } else { + self.upper_bound(raw_bytes::from_slice(item)) + } + } + /// Merges another sketch into this one. /// /// # Errors @@ -371,12 +479,13 @@ impl CountMinSketch { /// Serializes this sketch into the Count-Min binary format. /// - /// New sketches use serial version 2. Sketches deserialized from version 1 images retain the - /// legacy Rust item-to-counter mapping and are serialized as version 1 again. + /// Preserves the sketch's serial version. The default constructors create version 1 sketches; + /// [`Self::new_v2`] and [`Self::with_seed_v2`] create version 2 sketches. Deserializing an + /// image retains its version and hashing behavior for queries, updates, merges, and + /// serialization. /// - /// Version 2 uses the portable DataSketches Count-Min format and [`CountMinItem`] encoding. - /// Use `i64` counters for cross-language interchange; other counter types are compatible only - /// when their eight-byte values can be interpreted as signed 64-bit integers by the reader. + /// Version 1 images are not portable across implementations. Version 2 supports cross-language + /// interchange with `i64` counters and the input encodings documented on [`CountMinSketch`]. /// /// # Examples /// @@ -427,7 +536,7 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the default seed. /// - /// Reads portable version 2 images using [`CountMinItem`] encoding. Counter values must fit + /// Reads version 2 images with the encodings documented on [`CountMinSketch`]. Values must fit /// `T`. /// /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; @@ -456,7 +565,7 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the provided seed. /// - /// Reads portable version 2 images using [`CountMinItem`] encoding. Counter values must fit + /// Reads version 2 images with the encodings documented on [`CountMinSketch`]. Values must fit /// `T`. /// /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; @@ -587,6 +696,24 @@ impl CountMinSketch { self.counts.capacity() * size_of::() + self.hash_seeds.capacity() * size_of::() } + fn create( + num_hashes: u8, + num_buckets: u32, + seed: u64, + serial_version: u8, + ) -> Result { + let entries = entries_for_config(num_hashes, num_buckets)?; + let seed_hash = compute_seed_hash(seed, ErrorKind::InvalidArgument)?; + Ok(Self::make( + num_hashes, + num_buckets, + seed, + seed_hash, + serial_version, + entries, + )) + } + fn make( num_hashes: u8, num_buckets: u32, @@ -609,14 +736,18 @@ impl CountMinSketch { } } - fn bucket_index(&self, item: &I, seed: u64) -> usize { + fn bucket_index(&self, item: &I, seed: u64) -> usize { let hash = match self.serial_version { SERIAL_VERSION_1 => { let mut hasher = MurmurHash3X64128::with_seed(seed); item.hash(&mut hasher); hasher.finish128().0 } - SERIAL_VERSION_2 => item.hash_v2(seed) >> 1, + SERIAL_VERSION_2 => { + let mut hasher = V2Hasher(XxHash64::with_seed(seed)); + item.hash(&mut hasher); + hasher.finish() >> 1 + } _ => unreachable!("unsupported Count-Min serial version"), }; self.buckets.index(hash) @@ -681,8 +812,8 @@ impl CountMinSketch { } fn entries_for_config(num_hashes: u8, num_buckets: u32) -> Result { - if !(1..=127).contains(&num_hashes) { - return Err(Error::invalid_argument("num_hashes must be within 1..=127")); + if num_hashes == 0 { + return Err(Error::invalid_argument("num_hashes must be at least 1")); } if num_buckets < 3 { return Err(Error::invalid_argument("num_buckets must be at least 3")); diff --git a/datasketches/src/hash/value/mod.rs b/datasketches/src/hash/value/mod.rs index 175412bf..a4867a3d 100644 --- a/datasketches/src/hash/value/mod.rs +++ b/datasketches/src/hash/value/mod.rs @@ -17,10 +17,9 @@ //! Hashable value wrappers for sketches. //! -//! Most sketch update APIs accept any value that implements [`Hash`]. For most Rust values, +//! Sketch update APIs accept any value that implements [`Hash`]. For most Rust values, //! passing the value directly is sufficient. This module provides value wrappers for //! cases where the default implementation does not match a sketch's compatibility rules. -//! Count-Min instead accepts its own set of integer, string, and byte inputs directly. //! //! ## Floating-point Numbers //! diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs index b57c6b80..1108eaf0 100644 --- a/tests-integration/tests/countmin_test/items.rs +++ b/tests-integration/tests/countmin_test/items.rs @@ -15,11 +15,15 @@ // specific language governing permissions and limitations // under the License. -use datasketches::countmin::CountMinItem; +use std::hash::Hash; + use datasketches::countmin::CountMinSketch; +use datasketches::error::ErrorKind; +use datasketches::hash::value::natural_extend; +use datasketches::hash::value::raw_bytes; -fn assert_same_item(left: I, right: J) { - let mut a = CountMinSketch::::new(3, 1000).unwrap(); +fn assert_same_item(left: I, right: J) { + let mut a = CountMinSketch::::new_v2(3, 1000).unwrap(); let mut b = a.clone(); a.update_with_weight(&left, 7); b.update_with_weight(&right, 7); @@ -29,56 +33,85 @@ fn assert_same_item(left: I, right: J) { } #[test] -fn integer_widths_use_the_same_64_bit_encoding() { - assert_same_item(-1i8, -1i64); - assert_same_item(-1i16, -1i64); - assert_same_item(-1i32, -1i64); - assert_same_item(-1isize, -1i64); - assert_same_item(u8::MAX, 255u64); - assert_same_item(u16::MAX, 65535u64); - assert_same_item(u32::MAX, u32::MAX as u64); - assert_same_item(1usize, 1u64); - assert_same_item(1i32, 1u64); +fn v2_integer_wrappers_use_little_endian_64_bit_encoding() { + assert_same_item(natural_extend::from_i8(-1), -1i64); + assert_same_item(natural_extend::from_i16(-1), -1i64); + assert_same_item(natural_extend::from_i32(-1), -1i64); + assert_same_item(natural_extend::from_u8(255), 255u64); + assert_same_item(natural_extend::from_u16(65535), 65535u64); + assert_same_item(natural_extend::from_u32(u32::MAX), u32::MAX as u64); assert_same_item(-1i64, u64::MAX); assert_same_item(i64::MIN, 1u64 << 63); - assert_same_item(0x0102_0304_0506_0708u64, [8u8, 7, 6, 5, 4, 3, 2, 1]); + assert_same_item(-1i64, raw_bytes::from_slice(&[0xff; 8])); + assert_same_item( + 0x0102_0304_0506_0708u64, + raw_bytes::from_slice(&[8, 7, 6, 5, 4, 3, 2, 1]), + ); } #[test] -fn owned_and_borrowed_strings_and_bytes_identify_the_same_item() { +fn raw_byte_methods_match_wrappers_for_both_versions() { let text = "été and a long UTF-8 string crossing the 32-byte hash block boundary"; - assert_same_item(text, text.to_owned()); - assert_same_item(text, text.as_bytes()); - assert_same_item(text, text.as_bytes().to_vec()); - assert_same_item("apple", *b"apple"); - assert_same_item("apple", b"apple"); - assert_same_item("été", [0xc3u8, 0xa9, 0x74, 0xc3, 0xa9]); + for mut sketch in [ + CountMinSketch::::new(3, 1000).unwrap(), + CountMinSketch::::new_v2(3, 1000).unwrap(), + ] { + sketch.update_bytes_with_weight(text.as_bytes(), 7); + assert_eq!(sketch.estimate(raw_bytes::from_str(text)), 7); + assert_eq!(sketch.estimate_bytes(text.as_bytes()), 7); + assert_eq!( + sketch.lower_bound_bytes(text.as_bytes()), + sketch.lower_bound(raw_bytes::from_str(text)) + ); + assert_eq!( + sketch.upper_bound_bytes(text.as_bytes()), + sketch.upper_bound(raw_bytes::from_str(text)) + ); + sketch.update(raw_bytes::from_string(text.to_owned())); + sketch.update(raw_bytes::from_vec(text.as_bytes().to_vec())); + assert_eq!(sketch.estimate_bytes(text.as_bytes()), 9); + } } #[test] -fn empty_items_are_ignored_even_in_a_full_sketch() { - fn check(sketch: &mut CountMinSketch, item: I) { - let before = sketch.clone(); - sketch.update(&item); - sketch.update_with_weight(&item, i64::MIN); - assert_eq!(*sketch, before); - assert_eq!(sketch.estimate(&item), 0); - assert_eq!(sketch.lower_bound(&item), 0); - assert_eq!(sketch.upper_bound(item), 0); - } - +fn byte_methods_ignore_empty_inputs_even_in_a_full_sketch() { for filled in [false, true] { - let mut sketch = CountMinSketch::::new(1, 3).unwrap(); - if filled { - for i in 0..100 { - sketch.update(i); + for mut sketch in [ + CountMinSketch::::new(1, 3).unwrap(), + CountMinSketch::::new_v2(1, 3).unwrap(), + ] { + if filled { + for i in 0..100u64 { + sketch.update(i); + } } + let before = sketch.clone(); + sketch.update_bytes(b""); + sketch.update_bytes_with_weight(b"", i64::MIN); + assert_eq!(sketch, before); + assert_eq!(sketch.estimate_bytes(b""), 0); + assert_eq!(sketch.lower_bound_bytes(b""), 0); + assert_eq!(sketch.upper_bound_bytes(b""), 0); } - check(&mut sketch, ""); - check(&mut sketch, String::new()); - check(&mut sketch, b"".as_slice()); - check(&mut sketch, [] as [u8; 0]); - check(&mut sketch, b""); - check(&mut sketch, Vec::::new()); } } + +#[test] +fn only_v2_constructors_limit_the_row_count_to_127() { + for rows in [128, 255] { + let legacy = CountMinSketch::::with_seed(rows, 3, 42).unwrap(); + assert_eq!(legacy.num_hashes(), rows); + assert_eq!( + CountMinSketch::::deserialize_with_seed(&legacy.serialize(), 42).unwrap(), + legacy + ); + assert_eq!( + CountMinSketch::::with_seed_v2(rows, 3, 42) + .unwrap_err() + .kind(), + ErrorKind::InvalidArgument + ); + } + assert!(CountMinSketch::::new_v2(127, 3).is_ok()); + assert!(CountMinSketch::::new_v2(0, 3).is_err()); +} diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index 8710c1cf..b85a8876 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -415,14 +415,8 @@ fn test_truncated_non_empty_payload_is_rejected_before_table_allocation() { #[test] fn test_invalid_hashes_return_error() { - for rows in [0, 128, 255] { - let error = CountMinSketch::::new(rows, 5).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); - } - assert_eq!( - CountMinSketch::::new(127, 3).unwrap().num_hashes(), - 127 - ); + let error = CountMinSketch::::new(0, 5).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); } #[test] diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index 1cd65e51..f2c09739 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -16,10 +16,13 @@ // under the License. use std::fs; +use std::hash::Hash; +use std::hash::Hasher; -use datasketches::countmin::CountMinItem; use datasketches::countmin::CountMinSketch; use datasketches::error::ErrorKind; +use datasketches::hash::value::natural_extend; +use datasketches::hash::value::raw_bytes; use googletest::assert_that; use googletest::prelude::contains_substring; use tests_integration::ZERO_HASH_SEED; @@ -53,34 +56,34 @@ fn v2_reads_and_writes_the_published_image() { 0, ]; let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); - let mut sketch = CountMinSketch::::new(2, 9).unwrap(); + let mut sketch = CountMinSketch::::new_v2(2, 9).unwrap(); let mut empty = bytes[..16].to_vec(); empty[3] = 1; assert_eq!(sketch.serialize(), empty); assert_eq!(CountMinSketch::::deserialize(&empty).unwrap(), sketch); - sketch.update_with_weight(1, 10); - sketch.update_with_weight(9, 810); - sketch.update_with_weight("apple", 5); + sketch.update_with_weight(1i64, 10); + sketch.update_with_weight(9i64, 810); + sketch.update_bytes_with_weight(b"apple", 5); assert_eq!(sketch.serialize(), bytes); let mut restored = CountMinSketch::::deserialize(&bytes).unwrap(); assert_eq!(restored, sketch); assert_eq!(restored.estimate(1u64), 10); - assert_eq!(restored.estimate(9i8), 810); - assert_eq!(restored.estimate(b"apple"), 5); - restored.update_with_weight(b"apple".as_slice(), -2); - assert_eq!(restored.estimate("apple"), 3); + assert_eq!(restored.estimate(natural_extend::from_i8(9)), 810); + assert_eq!(restored.estimate_bytes(b"apple"), 5); + restored.update_bytes_with_weight(b"apple", -2); + assert_eq!(restored.estimate(raw_bytes::from_str("apple")), 3); assert_eq!(restored.total_weight(), 827); sketch.merge(&restored).unwrap(); - assert_eq!(sketch.estimate("apple"), 8); + assert_eq!(sketch.estimate(raw_bytes::from_str("apple")), 8); assert_eq!(sketch.total_weight(), 1652); } #[test] fn v2_matches_published_string_and_signed_integer_buckets() { - fn check(item: I, expected: [usize; 3]) { - let mut sketch = CountMinSketch::::new(3, 1000).unwrap(); + fn check(item: I, expected: [usize; 3]) { + let mut sketch = CountMinSketch::::new_v2(3, 1000).unwrap(); sketch.update_with_weight(&item, 7); let bytes = sketch.serialize(); for (row, bucket) in expected.into_iter().enumerate() { @@ -100,10 +103,10 @@ fn v2_matches_published_string_and_signed_integer_buckets() { // Values computed with DataSketches Java XxHash64 in TCK discussion #18, section 7.3. check(-1i64, [256, 641, 308]); check(i64::MIN, [951, 0, 298]); - check("apple", [130, 747, 144]); - check("été", [806, 491, 163]); - check(b"apple".as_slice(), [130, 747, 144]); - check("été".as_bytes(), [806, 491, 163]); + check(raw_bytes::from_str("apple"), [130, 747, 144]); + check(raw_bytes::from_str("été"), [806, 491, 163]); + check(raw_bytes::from_slice(b"apple"), [130, 747, 144]); + check(raw_bytes::from_slice("été".as_bytes()), [806, 491, 163]); } #[test] @@ -188,9 +191,69 @@ fn legacy_strings_bytes_and_narrow_integers_keep_their_hash_encoding() { ); } +#[test] +fn default_sketch_preserves_legacy_wrappers_and_custom_hash_inputs() { + struct Key(u64); + + impl Hash for Key { + fn hash(&self, state: &mut H) { + self.0.hash(state); + } + } + + // Generated by Rust v0.6.0 with the same wrappers, custom key, and updates below. + let words: [u64; 21] = [ + 1179906, + 41601130538467337, + 36, + 7, + 0, + 16, + 0, + 13, + 0, + 0, + 0, + 0, + 11, + 0, + 12, + 0, + 0, + 0, + 13, + 0, + 0, + ]; + let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); + let mut sketch = CountMinSketch::::new(2, 9).unwrap(); + sketch.update_with_weight(raw_bytes::from_str("apple"), 5); + sketch.update_with_weight(natural_extend::from_i8(-1), 7); + sketch.update_with_weight(Key(42), 11); + sketch.update_with_weight(raw_bytes::from_str(""), 13); + assert_eq!(sketch.serialize(), bytes); + + let restored = CountMinSketch::::deserialize(&bytes).unwrap(); + assert_eq!(restored.estimate(raw_bytes::from_str("apple")), 12); + assert_eq!(restored.estimate(natural_extend::from_i8(-1)), 7); + assert_eq!(restored.estimate(Key(42)), 11); + assert_eq!(restored.estimate(raw_bytes::from_str("")), 13); + + sketch.merge(&restored).unwrap(); + sketch.update(Key(42)); + assert_eq!(sketch.estimate(Key(42)), 23); + assert_eq!(sketch.lower_bound(Key(42)), 23); + assert!(sketch.upper_bound(Key(42)) >= 23); + assert_eq!(sketch.total_weight(), 73); + assert_eq!( + CountMinSketch::::deserialize(&sketch.serialize()).unwrap(), + sketch + ); +} + #[test] fn v2_row_limit_does_not_reject_legacy_images() { - let mut bytes = CountMinSketch::::new(1, 3).unwrap().serialize(); + let mut bytes = CountMinSketch::::new_v2(1, 3).unwrap().serialize(); bytes[12] = 128; assert_eq!( CountMinSketch::::deserialize(&bytes) @@ -224,8 +287,9 @@ fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { assert_eq!(sketch.estimate(1u64), 15); assert_eq!(sketch.serialize()[1], 1); - let mut merged = - CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + let mut merged = CountMinSketch::::with_seed(1, 5, 123).unwrap(); + merged.update_with_weight(1u64, 10); + assert_eq!(merged.serialize(), RUST_V1_SERIALIZED); merged.merge(&sketch).unwrap(); assert_eq!(merged.total_weight(), 25); assert_eq!(merged.estimate(1u64), 25); @@ -234,7 +298,7 @@ fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { #[test] fn rejects_merge_across_serial_versions() { let legacy = CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); - let mut current = CountMinSketch::::with_seed(1, 5, 123).unwrap(); + let mut current = CountMinSketch::::with_seed_v2(1, 5, 123).unwrap(); current.update_with_weight(1u64, 10); let before = current.clone(); @@ -340,7 +404,7 @@ fn test_v2_known_answer_vectors() { (9001, [185, 898, 4], [490, 217, 440]), (42, [468, 459, 696], [841, 428, 322]), ] { - let mut sketch = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); + let mut sketch = CountMinSketch::::with_seed_v2(3, NUM_BUCKETS as u32, seed).unwrap(); sketch.update_with_weight(1u64, 10); sketch.update_with_weight(9u64, 810); @@ -372,7 +436,7 @@ fn test_v2_known_answer_vectors() { decoded.update_with_weight(1u64, 5); assert_eq!(decoded.estimate(1u64), 15); - let mut merged = CountMinSketch::::with_seed(3, NUM_BUCKETS as u32, seed).unwrap(); + let mut merged = CountMinSketch::::with_seed_v2(3, NUM_BUCKETS as u32, seed).unwrap(); merged.merge(&decoded).unwrap(); assert_eq!(merged.total_weight(), 825); assert_eq!(merged.estimate(1u64), 15); From fbf4b664ab0ef305f1001775a3a065145e7e6384 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 14:48:52 +0800 Subject: [PATCH 04/13] refactor(countmin): reuse value wrappers and consolidate compatibility tests --- CHANGELOG.md | 1 - datasketches/src/countmin/sketch.rs | 64 +------------ .../tests/countmin_test/items.rs | 62 +----------- .../tests/serde_tests/countmin.rs | 95 +++++-------------- 4 files changed, 32 insertions(+), 190 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fa9f86d..2dacbf6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,6 @@ All significant changes to this project will be documented in this file. ### New features * Add opt-in Count-Min serial version 2 through `CountMinSketch::new_v2` and `with_seed_v2`, with portable row seeds, item hashing, and bucket mapping. Default constructors and version 1 images retain their existing hashing and serialization behavior, including custom `Hash` inputs and value wrappers. Version 2 interchange requires the documented input encodings; sketches of different versions cannot be merged. -* Add raw-byte update and query methods to `CountMinSketch`, with UTF-8 string support through `as_bytes()` and automatic empty-input handling. * Add `HllUnion::serialize` to serialize union results directly. * Add `XorFilter` and `XorFilterBuilder` behind the `xor` feature for immutable membership queries on values or precomputed hashes. Filters support 8-, 16-, and 32-bit fingerprints and can be serialized for later queries. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index b77e9561..6c2918e3 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -37,7 +37,6 @@ use crate::hash::MurmurHash3X64128; use crate::hash::XxHash64; use crate::hash::check_seed_hash; use crate::hash::compute_seed_hash; -use crate::hash::value::raw_bytes; const MAX_TABLE_ENTRIES: usize = 1 << 30; @@ -75,9 +74,8 @@ impl Hasher for V2Hasher { /// * `i64` and `u64` are hashed as eight little-endian bytes. Widen smaller integers first, or use /// [`crate::hash::value::natural_extend`]. /// * Strings use UTF-8 and byte arrays use their contents without prefixes or terminators. Use -/// [`Self::update_bytes`] and [`Self::estimate_bytes`], or [`raw_bytes`] wrappers. -/// * The byte methods ignore empty inputs. With wrappers, skip empty inputs before updating and -/// return zero for empty queries. +/// [`crate::hash::value::raw_bytes`] wrappers. +/// * Callers must skip empty string/byte updates and return zero for empty queries. /// /// Generic methods retain the supplied [`Hash`] encoding, including for empty inputs. Bare /// strings, slices, and arbitrary custom `Hash` implementations are not automatically portable. @@ -195,7 +193,7 @@ impl CountMinSketch { /// use datasketches::hash::value::raw_bytes; /// /// let mut sketch = CountMinSketch::::new_v2(3, 1000)?; - /// sketch.update_bytes("apple".as_bytes()); + /// sketch.update(raw_bytes::from_str("apple")); /// assert_eq!(sketch.estimate(raw_bytes::from_str("apple")), 1); /// # Ok::<(), datasketches::error::Error>(()) /// ``` @@ -379,62 +377,6 @@ impl CountMinSketch { estimate.checked_add(error).unwrap_or(T::MAX) } - /// Updates the sketch with one occurrence of raw bytes, ignoring an empty input. - /// - /// Equivalent to updating with [`raw_bytes::from_slice`] for non-empty inputs. Strings can - /// be passed as UTF-8 with `str::as_bytes`. Uses the sketch's current hash version. - /// - /// # Panics - /// - /// Panics without modifying the sketch if the total absolute weight would exceed `T::MAX`. - pub fn update_bytes(&mut self, item: &[u8]) { - self.update_bytes_with_weight(item, T::ONE); - } - - /// Updates the sketch with raw bytes and a weight, ignoring an empty input regardless of - /// weight. - /// - /// Uses the same encoding as [`Self::update_bytes`]. - /// - /// # Panics - /// - /// Panics without modifying the sketch if the absolute weight or the total absolute weight - /// cannot be represented by `T`. - pub fn update_bytes_with_weight(&mut self, item: &[u8], weight: T) { - if !item.is_empty() { - self.update_with_weight(raw_bytes::from_slice(item), weight); - } - } - - /// Returns the estimated frequency of raw bytes, or zero for an empty input. - /// - /// Uses the same encoding as [`Self::update_bytes`]. - pub fn estimate_bytes(&self, item: &[u8]) -> T { - if item.is_empty() { - T::ZERO - } else { - self.estimate(raw_bytes::from_slice(item)) - } - } - - /// Returns the lower frequency bound for raw bytes, or zero for an empty input. - /// - /// Uses the same encoding as [`Self::update_bytes`]. - pub fn lower_bound_bytes(&self, item: &[u8]) -> T { - self.estimate_bytes(item) - } - - /// Returns the upper frequency bound for raw bytes, or zero for an empty input. - /// - /// Uses the same encoding as [`Self::update_bytes`]. - pub fn upper_bound_bytes(&self, item: &[u8]) -> T { - if item.is_empty() { - T::ZERO - } else { - self.upper_bound(raw_bytes::from_slice(item)) - } - } - /// Merges another sketch into this one. /// /// # Errors diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs index 1108eaf0..30eb58a1 100644 --- a/tests-integration/tests/countmin_test/items.rs +++ b/tests-integration/tests/countmin_test/items.rs @@ -28,74 +28,22 @@ fn assert_same_item(left: I, right: J) { a.update_with_weight(&left, 7); b.update_with_weight(&right, 7); assert_eq!(a, b); - assert_eq!(a.estimate(right), 7); - assert_eq!(b.estimate(left), 7); } #[test] fn v2_integer_wrappers_use_little_endian_64_bit_encoding() { - assert_same_item(natural_extend::from_i8(-1), -1i64); - assert_same_item(natural_extend::from_i16(-1), -1i64); - assert_same_item(natural_extend::from_i32(-1), -1i64); + assert_same_item(natural_extend::from_i8(-2), -2i64); assert_same_item(natural_extend::from_u8(255), 255u64); - assert_same_item(natural_extend::from_u16(65535), 65535u64); - assert_same_item(natural_extend::from_u32(u32::MAX), u32::MAX as u64); - assert_same_item(-1i64, u64::MAX); - assert_same_item(i64::MIN, 1u64 << 63); - assert_same_item(-1i64, raw_bytes::from_slice(&[0xff; 8])); + assert_same_item( + -2i64, + raw_bytes::from_slice(&[0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]), + ); assert_same_item( 0x0102_0304_0506_0708u64, raw_bytes::from_slice(&[8, 7, 6, 5, 4, 3, 2, 1]), ); } -#[test] -fn raw_byte_methods_match_wrappers_for_both_versions() { - let text = "été and a long UTF-8 string crossing the 32-byte hash block boundary"; - for mut sketch in [ - CountMinSketch::::new(3, 1000).unwrap(), - CountMinSketch::::new_v2(3, 1000).unwrap(), - ] { - sketch.update_bytes_with_weight(text.as_bytes(), 7); - assert_eq!(sketch.estimate(raw_bytes::from_str(text)), 7); - assert_eq!(sketch.estimate_bytes(text.as_bytes()), 7); - assert_eq!( - sketch.lower_bound_bytes(text.as_bytes()), - sketch.lower_bound(raw_bytes::from_str(text)) - ); - assert_eq!( - sketch.upper_bound_bytes(text.as_bytes()), - sketch.upper_bound(raw_bytes::from_str(text)) - ); - sketch.update(raw_bytes::from_string(text.to_owned())); - sketch.update(raw_bytes::from_vec(text.as_bytes().to_vec())); - assert_eq!(sketch.estimate_bytes(text.as_bytes()), 9); - } -} - -#[test] -fn byte_methods_ignore_empty_inputs_even_in_a_full_sketch() { - for filled in [false, true] { - for mut sketch in [ - CountMinSketch::::new(1, 3).unwrap(), - CountMinSketch::::new_v2(1, 3).unwrap(), - ] { - if filled { - for i in 0..100u64 { - sketch.update(i); - } - } - let before = sketch.clone(); - sketch.update_bytes(b""); - sketch.update_bytes_with_weight(b"", i64::MIN); - assert_eq!(sketch, before); - assert_eq!(sketch.estimate_bytes(b""), 0); - assert_eq!(sketch.lower_bound_bytes(b""), 0); - assert_eq!(sketch.upper_bound_bytes(b""), 0); - } - } -} - #[test] fn only_v2_constructors_limit_the_row_count_to_127() { for rows in [128, 255] { diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index f2c09739..e6addac8 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -17,7 +17,6 @@ use std::fs; use std::hash::Hash; -use std::hash::Hasher; use datasketches::countmin::CountMinSketch; use datasketches::error::ErrorKind; @@ -64,15 +63,15 @@ fn v2_reads_and_writes_the_published_image() { sketch.update_with_weight(1i64, 10); sketch.update_with_weight(9i64, 810); - sketch.update_bytes_with_weight(b"apple", 5); + sketch.update_with_weight(raw_bytes::from_str("apple"), 5); assert_eq!(sketch.serialize(), bytes); let mut restored = CountMinSketch::::deserialize(&bytes).unwrap(); assert_eq!(restored, sketch); assert_eq!(restored.estimate(1u64), 10); assert_eq!(restored.estimate(natural_extend::from_i8(9)), 810); - assert_eq!(restored.estimate_bytes(b"apple"), 5); - restored.update_bytes_with_weight(b"apple", -2); + assert_eq!(restored.estimate(raw_bytes::from_str("apple")), 5); + restored.update_with_weight(raw_bytes::from_str("apple"), -2); assert_eq!(restored.estimate(raw_bytes::from_str("apple")), 3); assert_eq!(restored.total_weight(), 827); sketch.merge(&restored).unwrap(); @@ -81,9 +80,9 @@ fn v2_reads_and_writes_the_published_image() { } #[test] -fn v2_matches_published_string_and_signed_integer_buckets() { - fn check(item: I, expected: [usize; 3]) { - let mut sketch = CountMinSketch::::new_v2(3, 1000).unwrap(); +fn v2_matches_known_bucket_vectors() { + fn check(item: I, seed: u64, expected: [usize; 3]) { + let mut sketch = CountMinSketch::::with_seed_v2(3, 1000, seed).unwrap(); sketch.update_with_weight(&item, 7); let bytes = sketch.serialize(); for (row, bucket) in expected.into_iter().enumerate() { @@ -94,19 +93,27 @@ fn v2_matches_published_string_and_signed_integer_buckets() { .enumerate() .filter(|(_, count)| *count != 0) .collect(); - assert_eq!(counts, [(bucket, 7)]); + assert_eq!(counts, [(bucket, 7)], "seed {seed}, row {row}"); } - let restored = CountMinSketch::::deserialize(&bytes).unwrap(); + let restored = CountMinSketch::::deserialize_with_seed(&bytes, seed).unwrap(); assert_eq!(restored.estimate(item), 7); } - // Values computed with DataSketches Java XxHash64 in TCK discussion #18, section 7.3. - check(-1i64, [256, 641, 308]); - check(i64::MIN, [951, 0, 298]); - check(raw_bytes::from_str("apple"), [130, 747, 144]); - check(raw_bytes::from_str("été"), [806, 491, 163]); - check(raw_bytes::from_slice(b"apple"), [130, 747, 144]); - check(raw_bytes::from_slice("été".as_bytes()), [806, 491, 163]); + // Java XxHash64 vectors from TCK discussion #18, section 7.3. + check(-1i64, 9001, [256, 641, 308]); + check(i64::MIN, 9001, [951, 0, 298]); + check(raw_bytes::from_str("apple"), 9001, [130, 747, 144]); + check( + raw_bytes::from_slice("été".as_bytes()), + 9001, + [806, 491, 163], + ); + + // Independently computed with XXH64 over little-endian u64 inputs. + check(1u64, 9001, [185, 898, 4]); + check(9u64, 9001, [490, 217, 440]); + check(1u64, 42, [468, 459, 696]); + check(9u64, 42, [841, 428, 322]); } #[test] @@ -193,14 +200,9 @@ fn legacy_strings_bytes_and_narrow_integers_keep_their_hash_encoding() { #[test] fn default_sketch_preserves_legacy_wrappers_and_custom_hash_inputs() { + #[derive(Hash)] struct Key(u64); - impl Hash for Key { - fn hash(&self, state: &mut H) { - self.0.hash(state); - } - } - // Generated by Rust v0.6.0 with the same wrappers, custom key, and updates below. let words: [u64; 21] = [ 1179906, @@ -394,52 +396,3 @@ fn zero_seed_hash_uses_the_callers_error_kind() { CountMinSketch::::deserialize_with_seed(&bytes, ZERO_HASH_SEED).unwrap_err(); assert_eq!(deserialization_error.kind(), ErrorKind::InvalidData); } - -#[test] -fn test_v2_known_answer_vectors() { - // Independently computed with XXH64 over little-endian `u64` values. - const NUM_BUCKETS: usize = 1000; - - for (seed, one_buckets, nine_buckets) in [ - (9001, [185, 898, 4], [490, 217, 440]), - (42, [468, 459, 696], [841, 428, 322]), - ] { - let mut sketch = CountMinSketch::::with_seed_v2(3, NUM_BUCKETS as u32, seed).unwrap(); - sketch.update_with_weight(1u64, 10); - sketch.update_with_weight(9u64, 810); - - let bytes = sketch.serialize(); - assert_eq!(bytes[1], 2); - let counts: Vec = bytes[24..] - .chunks_exact(8) - .map(|bytes| u64::from_le_bytes(bytes.try_into().unwrap())) - .collect(); - assert_eq!(counts.len(), 3 * NUM_BUCKETS); - - for row in 0..3 { - let actual: Vec<(usize, u64)> = counts[row * NUM_BUCKETS..(row + 1) * NUM_BUCKETS] - .iter() - .copied() - .enumerate() - .filter(|(_, count)| *count != 0) - .collect(); - let mut expected = vec![(one_buckets[row], 10), (nine_buckets[row], 810)]; - expected.sort_unstable_by_key(|(bucket, _)| *bucket); - assert_eq!(actual, expected, "seed {seed}, row {row}"); - } - - let mut decoded = CountMinSketch::::deserialize_with_seed(&bytes, seed).unwrap(); - assert_eq!(decoded.total_weight(), 820); - assert_eq!(decoded.estimate(1u64), 10); - assert_eq!(decoded.estimate(9u64), 810); - assert_eq!(decoded.serialize(), bytes); - - decoded.update_with_weight(1u64, 5); - assert_eq!(decoded.estimate(1u64), 15); - let mut merged = CountMinSketch::::with_seed_v2(3, NUM_BUCKETS as u32, seed).unwrap(); - merged.merge(&decoded).unwrap(); - assert_eq!(merged.total_weight(), 825); - assert_eq!(merged.estimate(1u64), 15); - assert_eq!(merged.estimate(9u64), 810); - } -} From eff0b2e64c64cca4d5d496f70b9804a86949b767 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 15:07:23 +0800 Subject: [PATCH 05/13] fix(countmin): create version 2 sketches by default --- CHANGELOG.md | 2 +- datasketches/src/countmin/sketch.rs | 98 ++++--------------- .../tests/countmin_test/items.rs | 23 +---- .../tests/countmin_test/sketch.rs | 7 +- .../tests/serde_tests/countmin.rs | 48 +++++---- 5 files changed, 50 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dacbf6c..bcfdd93b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All significant changes to this project will be documented in this file. ### New features -* Add opt-in Count-Min serial version 2 through `CountMinSketch::new_v2` and `with_seed_v2`, with portable row seeds, item hashing, and bucket mapping. Default constructors and version 1 images retain their existing hashing and serialization behavior, including custom `Hash` inputs and value wrappers. Version 2 interchange requires the documented input encodings; sketches of different versions cannot be merged. +* Create new `CountMinSketch` instances with serial version 2 hashing for cross-language interchange using the documented input encodings. Version 1 images retain their legacy queries, updates, merges, and serialization. New sketches support 1–127 rows and cannot be merged with version 1 sketches or read by version-1-only libraries; rebuild from source items when migrating existing data to version 2. * Add `HllUnion::serialize` to serialize union results directly. * Add `XorFilter` and `XorFilterBuilder` behind the `xor` feature for immutable membership queries on values or precomputed hashes. Filters support 8-, 16-, and 32-bit fingerprints and can be serialized for later queries. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 6c2918e3..d58b76e9 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -66,9 +66,8 @@ impl Hasher for V2Hasher { /// /// The sketch provides upper and lower bounds on estimated item frequencies /// with configurable relative error and confidence. -/// [`Self::new`] and [`Self::with_seed`] create version 1 sketches, retaining Rust's existing -/// hashing behavior. [`Self::new_v2`] and [`Self::with_seed_v2`] opt into portable version 2 -/// hashing. Versions cannot be merged with each other. +/// New sketches use serial version 2. Deserialized version 1 sketches retain their legacy +/// hashing and serialization. Sketches of different versions cannot be merged. /// /// For version 2 cross-language interchange, use `i64` counters and encode items as follows: /// * `i64` and `u64` are hashed as eight little-endian bytes. Widen smaller integers first, or use @@ -135,47 +134,6 @@ impl BucketCount { } impl CountMinSketch { - /// Creates a version 1 CountMin sketch with the default seed. - /// - /// # Errors - /// - /// Returns an error if `num_hashes` is `0`, `num_buckets` is less than `3`, or - /// the total table size exceeds the supported limit. - /// - /// # Examples - /// - /// ``` - /// use datasketches::countmin::CountMinSketch; - /// - /// let sketch = CountMinSketch::::new(4, 128).unwrap(); - /// assert_eq!(sketch.num_buckets(), 128); - /// ``` - pub fn new(num_hashes: u8, num_buckets: u32) -> Result { - Self::with_seed(num_hashes, num_buckets, DEFAULT_UPDATE_SEED) - } - - /// Creates a version 1 CountMin sketch with the provided seed. - /// - /// # Errors - /// - /// Returns an error if any of: - /// * `num_hashes` is `0`. - /// * `num_buckets` is less than `3`. - /// * The total table size exceeds the supported limit. - /// * The computed seed hash is zero. - /// - /// # Examples - /// - /// ``` - /// use datasketches::countmin::CountMinSketch; - /// - /// let sketch = CountMinSketch::::with_seed(4, 64, 42).unwrap(); - /// assert_eq!(sketch.seed(), 42); - /// ``` - pub fn with_seed(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { - Self::create(num_hashes, num_buckets, seed, SERIAL_VERSION_1) - } - /// Creates a version 2 CountMin sketch with the default seed. /// /// See [`CountMinSketch`] for portable input encodings. Version 2 sketches cannot be merged @@ -192,13 +150,13 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// use datasketches::hash::value::raw_bytes; /// - /// let mut sketch = CountMinSketch::::new_v2(3, 1000)?; + /// let mut sketch = CountMinSketch::::new(3, 1000)?; /// sketch.update(raw_bytes::from_str("apple")); /// assert_eq!(sketch.estimate(raw_bytes::from_str("apple")), 1); /// # Ok::<(), datasketches::error::Error>(()) /// ``` - pub fn new_v2(num_hashes: u8, num_buckets: u32) -> Result { - Self::with_seed_v2(num_hashes, num_buckets, DEFAULT_UPDATE_SEED) + pub fn new(num_hashes: u8, num_buckets: u32) -> Result { + Self::with_seed(num_hashes, num_buckets, DEFAULT_UPDATE_SEED) } /// Creates a version 2 CountMin sketch with the provided seed. @@ -209,13 +167,17 @@ impl CountMinSketch { /// /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, the /// total table size exceeds the supported limit, or the computed seed hash is zero. - pub fn with_seed_v2(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { - if num_hashes > 127 { - return Err(Error::invalid_argument( - "version 2 num_hashes must be within 1..=127", - )); - } - Self::create(num_hashes, num_buckets, seed, SERIAL_VERSION_2) + pub fn with_seed(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { + let entries = entries_for_config(num_hashes, num_buckets)?; + let seed_hash = compute_seed_hash(seed, ErrorKind::InvalidArgument)?; + Ok(Self::make( + num_hashes, + num_buckets, + seed, + seed_hash, + SERIAL_VERSION_2, + entries, + )) } /// Returns the number of hash functions used by the sketch. @@ -421,10 +383,8 @@ impl CountMinSketch { /// Serializes this sketch into the Count-Min binary format. /// - /// Preserves the sketch's serial version. The default constructors create version 1 sketches; - /// [`Self::new_v2`] and [`Self::with_seed_v2`] create version 2 sketches. Deserializing an - /// image retains its version and hashing behavior for queries, updates, merges, and - /// serialization. + /// New sketches are written as version 2. A sketch deserialized from a version 1 image + /// retains its legacy hashing for queries and updates and is written back as version 1. /// /// Version 1 images are not portable across implementations. Version 2 supports cross-language /// interchange with `i64` counters and the input encodings documented on [`CountMinSketch`]. @@ -638,24 +598,6 @@ impl CountMinSketch { self.counts.capacity() * size_of::() + self.hash_seeds.capacity() * size_of::() } - fn create( - num_hashes: u8, - num_buckets: u32, - seed: u64, - serial_version: u8, - ) -> Result { - let entries = entries_for_config(num_hashes, num_buckets)?; - let seed_hash = compute_seed_hash(seed, ErrorKind::InvalidArgument)?; - Ok(Self::make( - num_hashes, - num_buckets, - seed, - seed_hash, - serial_version, - entries, - )) - } - fn make( num_hashes: u8, num_buckets: u32, @@ -754,8 +696,8 @@ impl CountMinSketch { } fn entries_for_config(num_hashes: u8, num_buckets: u32) -> Result { - if num_hashes == 0 { - return Err(Error::invalid_argument("num_hashes must be at least 1")); + if !(1..=127).contains(&num_hashes) { + return Err(Error::invalid_argument("num_hashes must be within 1..=127")); } if num_buckets < 3 { return Err(Error::invalid_argument("num_buckets must be at least 3")); diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs index 30eb58a1..423db896 100644 --- a/tests-integration/tests/countmin_test/items.rs +++ b/tests-integration/tests/countmin_test/items.rs @@ -18,12 +18,11 @@ use std::hash::Hash; use datasketches::countmin::CountMinSketch; -use datasketches::error::ErrorKind; use datasketches::hash::value::natural_extend; use datasketches::hash::value::raw_bytes; fn assert_same_item(left: I, right: J) { - let mut a = CountMinSketch::::new_v2(3, 1000).unwrap(); + let mut a = CountMinSketch::::new(3, 1000).unwrap(); let mut b = a.clone(); a.update_with_weight(&left, 7); b.update_with_weight(&right, 7); @@ -43,23 +42,3 @@ fn v2_integer_wrappers_use_little_endian_64_bit_encoding() { raw_bytes::from_slice(&[8, 7, 6, 5, 4, 3, 2, 1]), ); } - -#[test] -fn only_v2_constructors_limit_the_row_count_to_127() { - for rows in [128, 255] { - let legacy = CountMinSketch::::with_seed(rows, 3, 42).unwrap(); - assert_eq!(legacy.num_hashes(), rows); - assert_eq!( - CountMinSketch::::deserialize_with_seed(&legacy.serialize(), 42).unwrap(), - legacy - ); - assert_eq!( - CountMinSketch::::with_seed_v2(rows, 3, 42) - .unwrap_err() - .kind(), - ErrorKind::InvalidArgument - ); - } - assert!(CountMinSketch::::new_v2(127, 3).is_ok()); - assert!(CountMinSketch::::new_v2(0, 3).is_err()); -} diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index b85a8876..28b98a5a 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -415,8 +415,11 @@ fn test_truncated_non_empty_payload_is_rejected_before_table_allocation() { #[test] fn test_invalid_hashes_return_error() { - let error = CountMinSketch::::new(0, 5).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); + for rows in [0, 128, 255] { + let error = CountMinSketch::::new(rows, 5).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + assert!(CountMinSketch::::new(127, 5).is_ok()); } #[test] diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index e6addac8..8ea8dfa3 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -55,7 +55,7 @@ fn v2_reads_and_writes_the_published_image() { 0, ]; let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); - let mut sketch = CountMinSketch::::new_v2(2, 9).unwrap(); + let mut sketch = CountMinSketch::::new(2, 9).unwrap(); let mut empty = bytes[..16].to_vec(); empty[3] = 1; assert_eq!(sketch.serialize(), empty); @@ -82,7 +82,7 @@ fn v2_reads_and_writes_the_published_image() { #[test] fn v2_matches_known_bucket_vectors() { fn check(item: I, seed: u64, expected: [usize; 3]) { - let mut sketch = CountMinSketch::::with_seed_v2(3, 1000, seed).unwrap(); + let mut sketch = CountMinSketch::::with_seed(3, 1000, seed).unwrap(); sketch.update_with_weight(&item, 7); let bytes = sketch.serialize(); for (row, bucket) in expected.into_iter().enumerate() { @@ -199,11 +199,12 @@ fn legacy_strings_bytes_and_narrow_integers_keep_their_hash_encoding() { } #[test] -fn default_sketch_preserves_legacy_wrappers_and_custom_hash_inputs() { +fn legacy_image_preserves_wrappers_and_custom_hash_inputs() { #[derive(Hash)] struct Key(u64); - // Generated by Rust v0.6.0 with the same wrappers, custom key, and updates below. + // Generated by Rust v0.6.0: raw_bytes("apple", 5), natural_extend(-1i8, 7), + // Key(42, 11), and raw_bytes("", 13), with two rows and nine buckets. let words: [u64; 21] = [ 1179906, 41601130538467337, @@ -228,11 +229,7 @@ fn default_sketch_preserves_legacy_wrappers_and_custom_hash_inputs() { 0, ]; let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); - let mut sketch = CountMinSketch::::new(2, 9).unwrap(); - sketch.update_with_weight(raw_bytes::from_str("apple"), 5); - sketch.update_with_weight(natural_extend::from_i8(-1), 7); - sketch.update_with_weight(Key(42), 11); - sketch.update_with_weight(raw_bytes::from_str(""), 13); + let mut sketch = CountMinSketch::::deserialize(&bytes).unwrap(); assert_eq!(sketch.serialize(), bytes); let restored = CountMinSketch::::deserialize(&bytes).unwrap(); @@ -255,18 +252,20 @@ fn default_sketch_preserves_legacy_wrappers_and_custom_hash_inputs() { #[test] fn v2_row_limit_does_not_reject_legacy_images() { - let mut bytes = CountMinSketch::::new_v2(1, 3).unwrap().serialize(); - bytes[12] = 128; - assert_eq!( - CountMinSketch::::deserialize(&bytes) - .unwrap_err() - .kind(), - ErrorKind::InvalidData - ); - bytes[1] = 1; - let legacy = CountMinSketch::::deserialize(&bytes).unwrap(); - assert_eq!(legacy.num_hashes(), 128); - assert_eq!(legacy.serialize(), bytes); + for rows in [128, 255] { + let mut bytes = CountMinSketch::::new(1, 3).unwrap().serialize(); + bytes[12] = rows; + assert_eq!( + CountMinSketch::::deserialize(&bytes) + .unwrap_err() + .kind(), + ErrorKind::InvalidData + ); + bytes[1] = 1; + let legacy = CountMinSketch::::deserialize(&bytes).unwrap(); + assert_eq!(legacy.num_hashes(), rows); + assert_eq!(legacy.serialize(), bytes); + } } // Generated by CountMinSketch v0.5.0 with one row, five buckets, seed 123, and key 1 at weight 10. @@ -289,9 +288,8 @@ fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { assert_eq!(sketch.estimate(1u64), 15); assert_eq!(sketch.serialize()[1], 1); - let mut merged = CountMinSketch::::with_seed(1, 5, 123).unwrap(); - merged.update_with_weight(1u64, 10); - assert_eq!(merged.serialize(), RUST_V1_SERIALIZED); + let mut merged = + CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); merged.merge(&sketch).unwrap(); assert_eq!(merged.total_weight(), 25); assert_eq!(merged.estimate(1u64), 25); @@ -300,7 +298,7 @@ fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { #[test] fn rejects_merge_across_serial_versions() { let legacy = CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); - let mut current = CountMinSketch::::with_seed_v2(1, 5, 123).unwrap(); + let mut current = CountMinSketch::::with_seed(1, 5, 123).unwrap(); current.update_with_weight(1u64, 10); let before = current.clone(); From 122954518c045df3808aac5ea81bc08761ff4d51 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 21:53:29 +0800 Subject: [PATCH 06/13] fix(countmin): ignore empty item encodings --- datasketches/src/countmin/sketch.rs | 116 +++++++++++------- .../tests/countmin_test/items.rs | 25 ++++ 2 files changed, 95 insertions(+), 46 deletions(-) diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index d58b76e9..ac6de484 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -40,28 +40,6 @@ use crate::hash::compute_seed_hash; const MAX_TABLE_ENTRIES: usize = 1 << 30; -// V2 specifies little-endian integer inputs. Keep this policy local to Count-Min so other -// sketches and the V1 path retain their existing Hash encodings. -struct V2Hasher(XxHash64); - -impl Hasher for V2Hasher { - fn finish(&self) -> u64 { - self.0.finish64() - } - - fn write(&mut self, bytes: &[u8]) { - self.0.write(bytes); - } - - fn write_i64(&mut self, value: i64) { - self.write(&value.to_le_bytes()); - } - - fn write_u64(&mut self, value: u64) { - self.write(&value.to_le_bytes()); - } -} - /// CountMin sketch for estimating item frequencies. /// /// The sketch provides upper and lower bounds on estimated item frequencies @@ -74,11 +52,12 @@ impl Hasher for V2Hasher { /// [`crate::hash::value::natural_extend`]. /// * Strings use UTF-8 and byte arrays use their contents without prefixes or terminators. Use /// [`crate::hash::value::raw_bytes`] wrappers. -/// * Callers must skip empty string/byte updates and return zero for empty queries. /// -/// Generic methods retain the supplied [`Hash`] encoding, including for empty inputs. Bare -/// strings, slices, and arbitrary custom `Hash` implementations are not automatically portable. -/// Use the same encoding for updates and queries; version 1 data must keep its original encoding. +/// Version 2 ignores empty encodings: updates have no effect, and estimates and bounds are zero. +/// This includes empty `raw_bytes` wrappers and any [`Hash`] implementation that writes no bytes. +/// Bare strings, slices, and arbitrary custom `Hash` implementations are not automatically +/// portable. Use the same encoding for updates and queries; version 1 data must keep its original +/// encoding. #[derive(Debug, Clone, PartialEq)] pub struct CountMinSketch { num_hashes: u8, @@ -94,6 +73,33 @@ pub struct CountMinSketch { hash_seeds: Vec, } +// The portable format needs little-endian integer writes and an explicit empty-input check. +// XXH64 itself hashes empty inputs, so its output cannot identify whether counters apply. +// Legacy sketches bypass this adapter to keep their original item encodings. +struct ItemHasher { + hash: XxHash64, + empty: bool, +} + +impl Hasher for ItemHasher { + fn finish(&self) -> u64 { + self.hash.finish64() + } + + fn write(&mut self, bytes: &[u8]) { + self.empty &= bytes.is_empty(); + self.hash.write(bytes); + } + + fn write_i64(&mut self, value: i64) { + self.write(&value.to_le_bytes()); + } + + fn write_u64(&mut self, value: u64) { + self.write(&value.to_le_bytes()); + } +} + #[derive(Debug, Clone, PartialEq)] struct BucketCount { count: u32, @@ -272,6 +278,8 @@ impl CountMinSketch { /// Updates the sketch with the given item and weight. /// + /// Version 2 ignores empty encodings regardless of weight. See [`CountMinSketch`]. + /// /// # Panics /// /// Panics without modifying the sketch if the absolute weight or the total absolute weight @@ -290,13 +298,19 @@ impl CountMinSketch { if weight == T::ZERO { return; } - self.total_weight = weight - .checked_abs() - .and_then(|weight| self.total_weight.checked_add(weight)) - .expect("total absolute weight overflow"); let num_buckets = self.buckets.count as usize; for (row, seed) in self.hash_seeds.iter().enumerate() { - let bucket = self.bucket_index(&item, *seed); + let Some(bucket) = self.bucket_index(&item, *seed) else { + return; + }; + if row == 0 { + // Ignore empty encodings before checking weight, and check before changing + // counters. + self.total_weight = weight + .checked_abs() + .and_then(|weight| self.total_weight.checked_add(weight)) + .expect("total absolute weight overflow"); + } let index = row * num_buckets + bucket; self.counts[index] = self.counts[index] + weight; } @@ -314,17 +328,7 @@ impl CountMinSketch { /// assert!(sketch.estimate("pear") >= 2); /// ``` pub fn estimate(&self, item: I) -> T { - let num_buckets = self.buckets.count as usize; - let mut min = T::MAX; - for (row, seed) in self.hash_seeds.iter().enumerate() { - let bucket = self.bucket_index(&item, *seed); - let index = row * num_buckets + bucket; - let value = self.counts[index]; - if value < min { - min = value; - } - } - min + self.estimate_item(&item).unwrap_or(T::ZERO) } /// Returns the lower bound on the true frequency of the given item. @@ -334,7 +338,9 @@ impl CountMinSketch { /// Returns the upper bound on the true frequency of the given item. pub fn upper_bound(&self, item: I) -> T { - let estimate = self.estimate(item); + let Some(estimate) = self.estimate_item(&item) else { + return T::ZERO; + }; let error = self.total_weight.scale_nonnegative(self.relative_error()); estimate.checked_add(error).unwrap_or(T::MAX) } @@ -620,7 +626,19 @@ impl CountMinSketch { } } - fn bucket_index(&self, item: &I, seed: u64) -> usize { + // Preserve the distinction between an empty encoding and an item with a zero estimate: + // empty encodings also have a zero upper bound, without the usual error term. + fn estimate_item(&self, item: &I) -> Option { + let num_buckets = self.buckets.count as usize; + let mut min = T::MAX; + for (row, seed) in self.hash_seeds.iter().enumerate() { + let bucket = self.bucket_index(item, *seed)?; + min = min.min(self.counts[row * num_buckets + bucket]); + } + Some(min) + } + + fn bucket_index(&self, item: &I, seed: u64) -> Option { let hash = match self.serial_version { SERIAL_VERSION_1 => { let mut hasher = MurmurHash3X64128::with_seed(seed); @@ -628,13 +646,19 @@ impl CountMinSketch { hasher.finish128().0 } SERIAL_VERSION_2 => { - let mut hasher = V2Hasher(XxHash64::with_seed(seed)); + let mut hasher = ItemHasher { + hash: XxHash64::with_seed(seed), + empty: true, + }; item.hash(&mut hasher); + if hasher.empty { + return None; + } hasher.finish() >> 1 } _ => unreachable!("unsupported Count-Min serial version"), }; - self.buckets.index(hash) + Some(self.buckets.index(hash)) } } diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs index 423db896..1feb89c8 100644 --- a/tests-integration/tests/countmin_test/items.rs +++ b/tests-integration/tests/countmin_test/items.rs @@ -21,6 +21,31 @@ use datasketches::countmin::CountMinSketch; use datasketches::hash::value::natural_extend; use datasketches::hash::value::raw_bytes; +#[test] +fn empty_encodings_do_not_update_or_query_counters() { + fn check(sketch: &mut CountMinSketch, item: I) { + let before = sketch.clone(); + sketch.update_with_weight(&item, 5); + assert_eq!(*sketch, before); + sketch.update_with_weight(&item, i64::MIN); + assert_eq!(*sketch, before); + assert_eq!(sketch.estimate(&item), 0); + assert_eq!(sketch.lower_bound(&item), 0); + assert_eq!(sketch.upper_bound(&item), 0); + } + + let mut sketch = CountMinSketch::::new(1, 3).unwrap(); + for populated in [false, true] { + if populated { + // Integer 2 and the empty encoding share a bucket with these parameters. + sketch.update_with_weight(2u64, 5); + } + check(&mut sketch, raw_bytes::from_str("")); + check(&mut sketch, raw_bytes::from_slice(&[])); + check(&mut sketch, ()); + } +} + fn assert_same_item(left: I, right: J) { let mut a = CountMinSketch::::new(3, 1000).unwrap(); let mut b = a.clone(); From 5588bbc192b40d2e7a92c24baeb91aa79b81ab24 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 21:55:11 +0800 Subject: [PATCH 07/13] fix(countmin): widen integer inputs before hashing --- datasketches/src/countmin/sketch.rs | 36 +++++++++++++++++-- .../tests/countmin_test/items.rs | 10 +++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index ac6de484..686172b1 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -48,8 +48,8 @@ const MAX_TABLE_ENTRIES: usize = 1 << 30; /// hashing and serialization. Sketches of different versions cannot be merged. /// /// For version 2 cross-language interchange, use `i64` counters and encode items as follows: -/// * `i64` and `u64` are hashed as eight little-endian bytes. Widen smaller integers first, or use -/// [`crate::hash::value::natural_extend`]. +/// * Integers up to 64 bits, including `isize` and `usize`, are widened to 64 bits and hashed as +/// eight little-endian bytes. Signed integers are sign-extended. /// * Strings use UTF-8 and byte arrays use their contents without prefixes or terminators. Use /// [`crate::hash::value::raw_bytes`] wrappers. /// @@ -95,9 +95,41 @@ impl Hasher for ItemHasher { self.write(&value.to_le_bytes()); } + fn write_i8(&mut self, value: i8) { + self.write_i64(i64::from(value)); + } + + fn write_i16(&mut self, value: i16) { + self.write_i64(i64::from(value)); + } + + fn write_i32(&mut self, value: i32) { + self.write_i64(i64::from(value)); + } + + fn write_isize(&mut self, value: isize) { + self.write_i64(value as i64); + } + fn write_u64(&mut self, value: u64) { self.write(&value.to_le_bytes()); } + + fn write_u8(&mut self, value: u8) { + self.write_u64(u64::from(value)); + } + + fn write_u16(&mut self, value: u16) { + self.write_u64(u64::from(value)); + } + + fn write_u32(&mut self, value: u32) { + self.write_u64(u64::from(value)); + } + + fn write_usize(&mut self, value: usize) { + self.write_u64(value as u64); + } } #[derive(Debug, Clone, PartialEq)] diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs index 1feb89c8..472aedb0 100644 --- a/tests-integration/tests/countmin_test/items.rs +++ b/tests-integration/tests/countmin_test/items.rs @@ -55,7 +55,15 @@ fn assert_same_item(left: I, right: J) { } #[test] -fn v2_integer_wrappers_use_little_endian_64_bit_encoding() { +fn integer_inputs_use_little_endian_64_bit_encoding() { + assert_same_item(-2i8, -2i64); + assert_same_item(-2i16, -2i64); + assert_same_item(-2i32, -2i64); + assert_same_item(-2isize, -2i64); + assert_same_item(u8::MAX, u8::MAX as u64); + assert_same_item(u16::MAX, u16::MAX as u64); + assert_same_item(u32::MAX, u32::MAX as u64); + assert_same_item(usize::MAX, usize::MAX as u64); assert_same_item(natural_extend::from_i8(-2), -2i64); assert_same_item(natural_extend::from_u8(255), 255u64); assert_same_item( From 1a3c3eecdcc8f9354d30c09cbe92e6a0939a7dd8 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 7 Nov 2026 21:57:15 +0800 Subject: [PATCH 08/13] docs(countmin): demonstrate portable item encodings --- CHANGELOG.md | 2 +- datasketches/src/countmin/mod.rs | 9 ++--- datasketches/src/countmin/sketch.rs | 55 +++++++++++++---------------- 3 files changed, 30 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05c45913..10ba73c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All significant changes to this project will be documented in this file. ### New features -* Create new `CountMinSketch` instances with serial version 2 hashing for cross-language interchange using the documented input encodings. Version 1 images retain their legacy queries, updates, merges, and serialization. New sketches support 1–127 rows and cannot be merged with version 1 sketches or read by version-1-only libraries; rebuild from source items when migrating existing data to version 2. +* Use serial version 2 for new `CountMinSketch` instances, enabling cross-language interchange with the documented input encodings. New sketches support 1–127 rows and require a version 2 reader. Existing version 1 data remains readable and updatable, but must be rebuilt from source items to merge with new sketches. * Add `CpcUnion::serialize`, which produces the same bytes as `to_sketch().serialize()` and can be faster. * Add `HllUnion::serialize` to serialize union results directly. * Add `XorFilter` and `XorFilterBuilder` behind the `xor` feature for immutable membership queries on values or precomputed hashes. Filters support 8-, 16-, and 32-bit fingerprints and can be serialized for later queries. diff --git a/datasketches/src/countmin/mod.rs b/datasketches/src/countmin/mod.rs index d8e4f5de..57c67db8 100644 --- a/datasketches/src/countmin/mod.rs +++ b/datasketches/src/countmin/mod.rs @@ -24,11 +24,12 @@ //! //! ``` //! use datasketches::countmin::CountMinSketch; +//! use datasketches::hash::value::raw_bytes; //! //! let mut sketch = CountMinSketch::::new(5, 256).unwrap(); -//! sketch.update("apple"); -//! sketch.update_with_weight("banana", 3); -//! assert!(sketch.estimate("banana") >= 3); +//! sketch.update(raw_bytes::from_str("apple")); +//! sketch.update_with_weight(raw_bytes::from_str("banana"), 3); +//! assert!(sketch.estimate(raw_bytes::from_str("banana")) >= 3); //! ``` //! //! # Configuration Helpers @@ -39,7 +40,7 @@ //! let buckets = CountMinSketch::::suggest_num_buckets(0.01).unwrap(); //! let hashes = CountMinSketch::::suggest_num_hashes(0.99).unwrap(); //! let sketch = CountMinSketch::::new(hashes, buckets).unwrap(); -//! assert_eq!(sketch.estimate("apple"), 0); +//! assert_eq!(sketch.estimate(1i64), 0); //! ``` mod serialization; diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 686172b1..736f48c2 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -46,6 +46,7 @@ const MAX_TABLE_ENTRIES: usize = 1 << 30; /// with configurable relative error and confidence. /// New sketches use serial version 2. Deserialized version 1 sketches retain their legacy /// hashing and serialization. Sketches of different versions cannot be merged. +/// Version 1 images are not portable across implementations. /// /// For version 2 cross-language interchange, use `i64` counters and encode items as follows: /// * Integers up to 64 bits, including `isize` and `usize`, are widened to 64 bits and hashed as @@ -301,8 +302,8 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update("apple"); - /// assert!(sketch.estimate("apple") >= 1); + /// sketch.update(1i64); + /// assert!(sketch.estimate(1i64) >= 1); /// ``` pub fn update(&mut self, item: I) { self.update_with_weight(item, T::ONE); @@ -323,8 +324,8 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update_with_weight("banana", 3); - /// assert!(sketch.estimate("banana") >= 3); + /// sketch.update_with_weight(2i64, 3); + /// assert!(sketch.estimate(2i64) >= 3); /// ``` pub fn update_with_weight(&mut self, item: I, weight: T) { if weight == T::ZERO { @@ -356,8 +357,8 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update_with_weight("pear", 2); - /// assert!(sketch.estimate("pear") >= 2); + /// sketch.update_with_weight(3i64, 2); + /// assert!(sketch.estimate(3i64) >= 2); /// ``` pub fn estimate(&self, item: I) -> T { self.estimate_item(&item).unwrap_or(T::ZERO) @@ -393,11 +394,11 @@ impl CountMinSketch { /// let mut left = CountMinSketch::::new(4, 128).unwrap(); /// let mut right = CountMinSketch::::new(4, 128).unwrap(); /// - /// left.update("apple"); - /// right.update_with_weight("banana", 2); + /// left.update(1i64); + /// right.update_with_weight(2i64, 2); /// /// left.merge(&right).unwrap(); - /// assert!(left.estimate("banana") >= 2); + /// assert!(left.estimate(2i64) >= 2); /// ``` pub fn merge(&mut self, other: &CountMinSketch) -> Result<(), Error> { if self.num_hashes != other.num_hashes @@ -433,10 +434,10 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update("apple"); + /// sketch.update(1i64); /// let bytes = sketch.serialize(); /// let decoded = CountMinSketch::::deserialize(&bytes).unwrap(); - /// assert!(decoded.estimate("apple") >= 1); + /// assert!(decoded.estimate(1i64) >= 1); /// ``` pub fn serialize(&self) -> Vec { let header_size = PREAMBLE_LONGS_SHORT as usize * LONG_SIZE_BYTES; @@ -476,12 +477,8 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the default seed. /// - /// Reads version 2 images with the encodings documented on [`CountMinSketch`]. Values must fit - /// `T`. - /// - /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; - /// queries, updates, and merges on version 1 images from other implementations remain - /// unreliable. + /// Accepts versions 1 and 2, preserving the image's hashing and serialization version. + /// Counter values must fit `T`. See [`CountMinSketch`] for input encodings and compatibility. /// /// # Errors /// @@ -494,10 +491,10 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 64).unwrap(); - /// sketch.update("apple"); + /// sketch.update(1i64); /// let bytes = sketch.serialize(); /// let decoded = CountMinSketch::::deserialize(&bytes).unwrap(); - /// assert!(decoded.estimate("apple") >= 1); + /// assert!(decoded.estimate(1i64) >= 1); /// ``` pub fn deserialize(bytes: &[u8]) -> Result { Self::deserialize_with_seed(bytes, DEFAULT_UPDATE_SEED) @@ -505,12 +502,8 @@ impl CountMinSketch { /// Deserializes a sketch from bytes using the provided seed. /// - /// Reads version 2 images with the encodings documented on [`CountMinSketch`]. Values must fit - /// `T`. - /// - /// Accepts serial versions 1 and 2. Version 1 uses the legacy Rust item-to-counter mapping; - /// queries, updates, and merges on version 1 images from other implementations remain - /// unreliable. + /// Accepts versions 1 and 2, preserving the image's hashing and serialization version. + /// Counter values must fit `T`. See [`CountMinSketch`] for input encodings and compatibility. /// /// # Errors /// @@ -523,10 +516,10 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::with_seed(4, 64, 7).unwrap(); - /// sketch.update("apple"); + /// sketch.update(1i64); /// let bytes = sketch.serialize(); /// let decoded = CountMinSketch::::deserialize_with_seed(&bytes, 7).unwrap(); - /// assert!(decoded.estimate("apple") >= 1); + /// assert!(decoded.estimate(1i64) >= 1); /// ``` pub fn deserialize_with_seed(bytes: &[u8], seed: u64) -> Result { fn read_value( @@ -703,9 +696,9 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update_with_weight("apple", 3); + /// sketch.update_with_weight(1i64, 3); /// sketch.halve(); - /// assert!(sketch.estimate("apple") >= 1); + /// assert!(sketch.estimate(1i64) >= 1); /// ``` pub fn halve(&mut self) { for c in &mut self.counts { @@ -731,9 +724,9 @@ impl CountMinSketch { /// use datasketches::countmin::CountMinSketch; /// /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update_with_weight("apple", 3); + /// sketch.update_with_weight(1i64, 3); /// sketch.decay(0.5); - /// assert!(sketch.estimate("apple") >= 1); + /// assert!(sketch.estimate(1i64) >= 1); /// ``` pub fn decay(&mut self, decay: f64) { assert!(decay > 0.0 && decay <= 1.0, "decay must be within (0, 1]"); From abca6533dfcef20cb1ee5dd95e8cdf520ba291fd Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 8 Nov 2026 09:34:23 +0800 Subject: [PATCH 09/13] test(countmin): remove duplicate compatibility assertions --- tests-integration/tests/serde_tests/countmin.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index 8ea8dfa3..f60d1cfe 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -95,8 +95,6 @@ fn v2_matches_known_bucket_vectors() { .collect(); assert_eq!(counts, [(bucket, 7)], "seed {seed}, row {row}"); } - let restored = CountMinSketch::::deserialize_with_seed(&bytes, seed).unwrap(); - assert_eq!(restored.estimate(item), 7); } // Java XxHash64 vectors from TCK discussion #18, section 7.3. @@ -241,8 +239,6 @@ fn legacy_image_preserves_wrappers_and_custom_hash_inputs() { sketch.merge(&restored).unwrap(); sketch.update(Key(42)); assert_eq!(sketch.estimate(Key(42)), 23); - assert_eq!(sketch.lower_bound(Key(42)), 23); - assert!(sketch.upper_bound(Key(42)) >= 23); assert_eq!(sketch.total_weight(), 73); assert_eq!( CountMinSketch::::deserialize(&sketch.serialize()).unwrap(), From f74fbe1beaf746b97538781789fb8d467430409d Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 8 Nov 2026 16:40:54 +0800 Subject: [PATCH 10/13] refactor(countmin): align item hashing with hash::value wrappers `ItemHasher` overrode `Hasher::write_i8` through `write_usize` to widen narrow integers to 64 bits and tracked zero-byte writes to skip empty encodings. Because `std::hash::Hasher` integer methods are low-level stream operations used by standard `Hash` implementations (for example, `str::hash` writes a trailing `0xff` via `write_u8` and `[T]::hash` writes the slice length via `write_usize`), overriding them alters the encoding of ordinary Rust strings, slices, booleans, enums, and derived composite keys. Deferring weight validation until after the first row's hash check also allowed invalid weights such as `i64::MIN` to be silently ignored on zero-byte inputs. Hash version 2 items directly with `XxHash64`, matching `BloomFilter`, `XorFilter`, and the rest of the crate's `hash::value` design. Validate and accumulate the absolute update weight once before iterating over the sketch rows, document `natural_extend` for Count-Min sketches alongside Bloom filters, and fold the portable value-wrapper test into `countmin_test::sketch`. --- datasketches/src/countmin/sketch.rs | 113 +++--------------- datasketches/src/hash/value/mod.rs | 2 +- datasketches/src/hash/value/natural_extend.rs | 3 +- datasketches/src/lib.rs | 2 +- .../tests/countmin_test/items.rs | 77 ------------ tests-integration/tests/countmin_test/main.rs | 1 - .../tests/countmin_test/sketch.rs | 29 +++++ 7 files changed, 50 insertions(+), 177 deletions(-) delete mode 100644 tests-integration/tests/countmin_test/items.rs diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 736f48c2..f8778f50 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -74,65 +74,6 @@ pub struct CountMinSketch { hash_seeds: Vec, } -// The portable format needs little-endian integer writes and an explicit empty-input check. -// XXH64 itself hashes empty inputs, so its output cannot identify whether counters apply. -// Legacy sketches bypass this adapter to keep their original item encodings. -struct ItemHasher { - hash: XxHash64, - empty: bool, -} - -impl Hasher for ItemHasher { - fn finish(&self) -> u64 { - self.hash.finish64() - } - - fn write(&mut self, bytes: &[u8]) { - self.empty &= bytes.is_empty(); - self.hash.write(bytes); - } - - fn write_i64(&mut self, value: i64) { - self.write(&value.to_le_bytes()); - } - - fn write_i8(&mut self, value: i8) { - self.write_i64(i64::from(value)); - } - - fn write_i16(&mut self, value: i16) { - self.write_i64(i64::from(value)); - } - - fn write_i32(&mut self, value: i32) { - self.write_i64(i64::from(value)); - } - - fn write_isize(&mut self, value: isize) { - self.write_i64(value as i64); - } - - fn write_u64(&mut self, value: u64) { - self.write(&value.to_le_bytes()); - } - - fn write_u8(&mut self, value: u8) { - self.write_u64(u64::from(value)); - } - - fn write_u16(&mut self, value: u16) { - self.write_u64(u64::from(value)); - } - - fn write_u32(&mut self, value: u32) { - self.write_u64(u64::from(value)); - } - - fn write_usize(&mut self, value: usize) { - self.write_u64(value as u64); - } -} - #[derive(Debug, Clone, PartialEq)] struct BucketCount { count: u32, @@ -331,19 +272,13 @@ impl CountMinSketch { if weight == T::ZERO { return; } + self.total_weight = weight + .checked_abs() + .and_then(|weight| self.total_weight.checked_add(weight)) + .expect("total absolute weight overflow"); let num_buckets = self.buckets.count as usize; for (row, seed) in self.hash_seeds.iter().enumerate() { - let Some(bucket) = self.bucket_index(&item, *seed) else { - return; - }; - if row == 0 { - // Ignore empty encodings before checking weight, and check before changing - // counters. - self.total_weight = weight - .checked_abs() - .and_then(|weight| self.total_weight.checked_add(weight)) - .expect("total absolute weight overflow"); - } + let bucket = self.bucket_index(&item, *seed); let index = row * num_buckets + bucket; self.counts[index] = self.counts[index] + weight; } @@ -361,7 +296,13 @@ impl CountMinSketch { /// assert!(sketch.estimate(3i64) >= 2); /// ``` pub fn estimate(&self, item: I) -> T { - self.estimate_item(&item).unwrap_or(T::ZERO) + let num_buckets = self.buckets.count as usize; + let mut min = T::MAX; + for (row, seed) in self.hash_seeds.iter().enumerate() { + let bucket = self.bucket_index(&item, *seed); + min = min.min(self.counts[row * num_buckets + bucket]); + } + min } /// Returns the lower bound on the true frequency of the given item. @@ -371,9 +312,7 @@ impl CountMinSketch { /// Returns the upper bound on the true frequency of the given item. pub fn upper_bound(&self, item: I) -> T { - let Some(estimate) = self.estimate_item(&item) else { - return T::ZERO; - }; + let estimate = self.estimate(item); let error = self.total_weight.scale_nonnegative(self.relative_error()); estimate.checked_add(error).unwrap_or(T::MAX) } @@ -651,19 +590,7 @@ impl CountMinSketch { } } - // Preserve the distinction between an empty encoding and an item with a zero estimate: - // empty encodings also have a zero upper bound, without the usual error term. - fn estimate_item(&self, item: &I) -> Option { - let num_buckets = self.buckets.count as usize; - let mut min = T::MAX; - for (row, seed) in self.hash_seeds.iter().enumerate() { - let bucket = self.bucket_index(item, *seed)?; - min = min.min(self.counts[row * num_buckets + bucket]); - } - Some(min) - } - - fn bucket_index(&self, item: &I, seed: u64) -> Option { + fn bucket_index(&self, item: &I, seed: u64) -> usize { let hash = match self.serial_version { SERIAL_VERSION_1 => { let mut hasher = MurmurHash3X64128::with_seed(seed); @@ -671,19 +598,13 @@ impl CountMinSketch { hasher.finish128().0 } SERIAL_VERSION_2 => { - let mut hasher = ItemHasher { - hash: XxHash64::with_seed(seed), - empty: true, - }; + let mut hasher = XxHash64::with_seed(seed); item.hash(&mut hasher); - if hasher.empty { - return None; - } - hasher.finish() >> 1 + hasher.finish64() >> 1 } _ => unreachable!("unsupported Count-Min serial version"), }; - Some(self.buckets.index(hash)) + self.buckets.index(hash) } } diff --git a/datasketches/src/hash/value/mod.rs b/datasketches/src/hash/value/mod.rs index a4867a3d..803aba5d 100644 --- a/datasketches/src/hash/value/mod.rs +++ b/datasketches/src/hash/value/mod.rs @@ -48,7 +48,7 @@ //! //! [`natural_extend::NaturalExtend`] first widens signed values to `i64` and unsigned values to //! `u64`, and then hashes the resulting integers. Use this encoding to match short-integer hashing -//! in DataSketches C++'s Bloom filters. +//! in DataSketches C++'s Bloom filters and Count-Min sketches. //! //! The concrete wrapper documentation provides more details and examples. //! diff --git a/datasketches/src/hash/value/natural_extend.rs b/datasketches/src/hash/value/natural_extend.rs index 29d7fcff..780116f6 100644 --- a/datasketches/src/hash/value/natural_extend.rs +++ b/datasketches/src/hash/value/natural_extend.rs @@ -20,7 +20,8 @@ //! [`NaturalExtend`] first widens signed values to `i64` and unsigned values to `u64`, and then //! hashes the resulting integers. //! -//! Use this encoding to match short-integer hashing in DataSketches C++'s Bloom filters. +//! Use this encoding to match short-integer hashing in DataSketches C++'s Bloom filters and +//! Count-Min sketches. use std::hash::Hash; use std::hash::Hasher; diff --git a/datasketches/src/lib.rs b/datasketches/src/lib.rs index 18cd2bc4..bc77ea81 100644 --- a/datasketches/src/lib.rs +++ b/datasketches/src/lib.rs @@ -54,7 +54,7 @@ //! * `raw_bytes` for byte and string contents; //! * `canonical_float` for floating-point values; //! * `sign_extend` for short integers used with HLL and CPC; -//! * `natural_extend` for short integers used with Bloom filters. +//! * `natural_extend` for short integers used with Bloom filters and Count-Min sketches. //! //! When interoperating with an implementation that skips empty strings, check for empty input //! before updating here as well. diff --git a/tests-integration/tests/countmin_test/items.rs b/tests-integration/tests/countmin_test/items.rs deleted file mode 100644 index 472aedb0..00000000 --- a/tests-integration/tests/countmin_test/items.rs +++ /dev/null @@ -1,77 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use std::hash::Hash; - -use datasketches::countmin::CountMinSketch; -use datasketches::hash::value::natural_extend; -use datasketches::hash::value::raw_bytes; - -#[test] -fn empty_encodings_do_not_update_or_query_counters() { - fn check(sketch: &mut CountMinSketch, item: I) { - let before = sketch.clone(); - sketch.update_with_weight(&item, 5); - assert_eq!(*sketch, before); - sketch.update_with_weight(&item, i64::MIN); - assert_eq!(*sketch, before); - assert_eq!(sketch.estimate(&item), 0); - assert_eq!(sketch.lower_bound(&item), 0); - assert_eq!(sketch.upper_bound(&item), 0); - } - - let mut sketch = CountMinSketch::::new(1, 3).unwrap(); - for populated in [false, true] { - if populated { - // Integer 2 and the empty encoding share a bucket with these parameters. - sketch.update_with_weight(2u64, 5); - } - check(&mut sketch, raw_bytes::from_str("")); - check(&mut sketch, raw_bytes::from_slice(&[])); - check(&mut sketch, ()); - } -} - -fn assert_same_item(left: I, right: J) { - let mut a = CountMinSketch::::new(3, 1000).unwrap(); - let mut b = a.clone(); - a.update_with_weight(&left, 7); - b.update_with_weight(&right, 7); - assert_eq!(a, b); -} - -#[test] -fn integer_inputs_use_little_endian_64_bit_encoding() { - assert_same_item(-2i8, -2i64); - assert_same_item(-2i16, -2i64); - assert_same_item(-2i32, -2i64); - assert_same_item(-2isize, -2i64); - assert_same_item(u8::MAX, u8::MAX as u64); - assert_same_item(u16::MAX, u16::MAX as u64); - assert_same_item(u32::MAX, u32::MAX as u64); - assert_same_item(usize::MAX, usize::MAX as u64); - assert_same_item(natural_extend::from_i8(-2), -2i64); - assert_same_item(natural_extend::from_u8(255), 255u64); - assert_same_item( - -2i64, - raw_bytes::from_slice(&[0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]), - ); - assert_same_item( - 0x0102_0304_0506_0708u64, - raw_bytes::from_slice(&[8, 7, 6, 5, 4, 3, 2, 1]), - ); -} diff --git a/tests-integration/tests/countmin_test/main.rs b/tests-integration/tests/countmin_test/main.rs index bf51e28c..825a6281 100644 --- a/tests-integration/tests/countmin_test/main.rs +++ b/tests-integration/tests/countmin_test/main.rs @@ -15,5 +15,4 @@ // specific language governing permissions and limitations // under the License. -mod items; mod sketch; diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index aef0f536..34372936 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -15,12 +15,15 @@ // specific language governing permissions and limitations // under the License. +use std::hash::Hash; use std::panic::AssertUnwindSafe; use std::panic::catch_unwind; use datasketches::countmin::CountMinSketch; use datasketches::countmin::UnsignedCountMinValue; use datasketches::error::ErrorKind; +use datasketches::hash::value::natural_extend; +use datasketches::hash::value::raw_bytes; use googletest::assert_that; use googletest::prelude::ge; use googletest::prelude::le; @@ -435,3 +438,29 @@ fn test_merge_incompatible() { let error = left.merge(&right).unwrap_err(); assert_eq!(error.kind(), ErrorKind::InvalidArgument); } + +#[test] +fn value_wrappers_match_portable_item_encodings() { + fn assert_same_item(left: I, right: J) { + let mut a = CountMinSketch::::new(3, 1000).unwrap(); + let mut b = a.clone(); + a.update_with_weight(&left, 7); + b.update_with_weight(&right, 7); + assert_eq!(a, b); + } + + assert_same_item(natural_extend::from_i8(-2), -2i64); + assert_same_item(natural_extend::from_i16(-2), -2i64); + assert_same_item(natural_extend::from_i32(-2), -2i64); + assert_same_item(natural_extend::from_u8(u8::MAX), u64::from(u8::MAX)); + assert_same_item(natural_extend::from_u16(u16::MAX), u64::from(u16::MAX)); + assert_same_item(natural_extend::from_u32(u32::MAX), u64::from(u32::MAX)); + assert_same_item( + -2i64, + raw_bytes::from_slice(&[0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]), + ); + assert_same_item( + 0x0102_0304_0506_0708u64, + raw_bytes::from_slice(&[8, 7, 6, 5, 4, 3, 2, 1]), + ); +} From fc2a9b68e5a9b8339e5076b1e65d652d27631b94 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 8 Nov 2026 16:44:17 +0800 Subject: [PATCH 11/13] refactor(countmin): model serial version as an enum and inline constants `CountMinSketch` stored `serial_version` as a raw `u8`, requiring `unreachable!()` arms when deriving row seeds and hashing items, and duplicated table dimension validation across constructor and deserializer helpers. Meanwhile, `countmin::serialization` held only five private constants used solely by `sketch.rs`. Introduce a private `SerialVersion` enum (`V1 = 1`, `V2 = 2`) that owns version decoding, row limits, row seed generation, table entry validation, and item hashing. Inline the preamble and version constants into `datasketches/src/countmin/sketch.rs` and remove `datasketches/src/countmin/serialization.rs`. --- datasketches/src/countmin/mod.rs | 2 - datasketches/src/countmin/serialization.rs | 22 --- datasketches/src/countmin/sketch.rs | 194 +++++++++++---------- 3 files changed, 105 insertions(+), 113 deletions(-) delete mode 100644 datasketches/src/countmin/serialization.rs diff --git a/datasketches/src/countmin/mod.rs b/datasketches/src/countmin/mod.rs index 57c67db8..40106225 100644 --- a/datasketches/src/countmin/mod.rs +++ b/datasketches/src/countmin/mod.rs @@ -43,8 +43,6 @@ //! assert_eq!(sketch.estimate(1i64), 0); //! ``` -mod serialization; - mod sketch; pub use self::sketch::CountMinSketch; diff --git a/datasketches/src/countmin/serialization.rs b/datasketches/src/countmin/serialization.rs deleted file mode 100644 index 6ec67456..00000000 --- a/datasketches/src/countmin/serialization.rs +++ /dev/null @@ -1,22 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -pub const PREAMBLE_LONGS_SHORT: u8 = 2; -pub const SERIAL_VERSION_1: u8 = 1; -pub const SERIAL_VERSION_2: u8 = 2; -pub const FLAGS_IS_EMPTY: u8 = 1 << 0; -pub const LONG_SIZE_BYTES: usize = 8; diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index f8778f50..d80f2aaa 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -25,11 +25,6 @@ use crate::codec::assert::insufficient_data; use crate::codec::family::Family; use crate::countmin::CountMinValue; use crate::countmin::UnsignedCountMinValue; -use crate::countmin::serialization::FLAGS_IS_EMPTY; -use crate::countmin::serialization::LONG_SIZE_BYTES; -use crate::countmin::serialization::PREAMBLE_LONGS_SHORT; -use crate::countmin::serialization::SERIAL_VERSION_1; -use crate::countmin::serialization::SERIAL_VERSION_2; use crate::error::Error; use crate::error::ErrorKind; use crate::hash::DEFAULT_UPDATE_SEED; @@ -38,6 +33,11 @@ use crate::hash::XxHash64; use crate::hash::check_seed_hash; use crate::hash::compute_seed_hash; +const PREAMBLE_LONGS_SHORT: u8 = 2; +const SERIAL_VERSION_1: u8 = 1; +const SERIAL_VERSION_2: u8 = 2; +const FLAGS_IS_EMPTY: u8 = 1 << 0; +const LONG_SIZE_BYTES: usize = 8; const MAX_TABLE_ENTRIES: usize = 1 << 30; /// CountMin sketch for estimating item frequencies. @@ -65,7 +65,7 @@ pub struct CountMinSketch { buckets: BucketCount, seed: u64, seed_hash: u16, - serial_version: u8, + serial_version: SerialVersion, // Every bucket satisfies |count| <= total_weight, so a checked total also bounds bucket // additions during update/merge: |a + b| <= |a| + |b|. Deserialization validates this // invariant; unsigned halving and decay preserve it. @@ -74,6 +74,91 @@ pub struct CountMinSketch { hash_seeds: Vec, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u8)] +enum SerialVersion { + V1 = SERIAL_VERSION_1, + V2 = SERIAL_VERSION_2, +} + +impl SerialVersion { + fn from_u8(byte: u8) -> Result { + match byte { + SERIAL_VERSION_1 => Ok(Self::V1), + SERIAL_VERSION_2 => Ok(Self::V2), + _ => Err(Error::deserial(format!( + "unsupported serial version: expected {SERIAL_VERSION_1} or {SERIAL_VERSION_2}, got {byte}" + ))), + } + } + + fn max_hashes(self) -> u8 { + match self { + Self::V1 => u8::MAX, + Self::V2 => 127, + } + } + + fn entries_for_config( + self, + num_hashes: u8, + num_buckets: u32, + kind: ErrorKind, + ) -> Result { + if !(1..=self.max_hashes()).contains(&num_hashes) { + return Err(Error::new( + kind, + match self { + Self::V1 => "num_hashes must be at least 1", + Self::V2 => "num_hashes must be within 1..=127", + }, + )); + } + if num_buckets < 3 { + return Err(Error::new(kind, "num_buckets must be at least 3")); + } + let entries = (num_hashes as usize) + .checked_mul(num_buckets as usize) + .ok_or_else(|| Error::new(kind, "num_hashes * num_buckets overflows usize"))?; + if entries >= MAX_TABLE_ENTRIES { + return Err(Error::new( + kind, + format!("num_hashes * num_buckets must be < {MAX_TABLE_ENTRIES}"), + )); + } + Ok(entries) + } + + fn make_hash_seeds(self, seed: u64, num_hashes: u8) -> Vec { + (0..num_hashes) + .map(|i| match self { + Self::V1 => { + let mut hasher = MurmurHash3X64128::with_seed(seed); + hasher.write(&u64::from(i).to_le_bytes()); + hasher.finish128().0 + } + Self::V2 => XxHash64::hash_u64(u64::from(i), seed), + }) + .collect() + } + + #[inline] + fn hash_item(self, item: &I, seed: u64) -> u64 { + match self { + Self::V1 => { + let mut hasher = MurmurHash3X64128::with_seed(seed); + item.hash(&mut hasher); + hasher.finish128().0 + } + Self::V2 => { + let mut hasher = XxHash64::with_seed(seed); + item.hash(&mut hasher); + hasher.finish64() >> 1 + } + } + } +} + #[derive(Debug, Clone, PartialEq)] struct BucketCount { count: u32, @@ -148,14 +233,18 @@ impl CountMinSketch { /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, the /// total table size exceeds the supported limit, or the computed seed hash is zero. pub fn with_seed(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { - let entries = entries_for_config(num_hashes, num_buckets)?; + let entries = SerialVersion::V2.entries_for_config( + num_hashes, + num_buckets, + ErrorKind::InvalidArgument, + )?; let seed_hash = compute_seed_hash(seed, ErrorKind::InvalidArgument)?; Ok(Self::make( num_hashes, num_buckets, seed, seed_hash, - SERIAL_VERSION_2, + SerialVersion::V2, entries, )) } @@ -389,7 +478,7 @@ impl CountMinSketch { let mut bytes = SketchBytes::with_capacity(header_size + payload_size); bytes.write_u8(PREAMBLE_LONGS_SHORT); - bytes.write_u8(self.serial_version); + bytes.write_u8(self.serial_version as u8); bytes.write_u8(Family::COUNTMIN.id); bytes.write_u8(if self.is_empty() { FLAGS_IS_EMPTY } else { 0 }); bytes.write_u32_le(0); // unused @@ -484,22 +573,13 @@ impl CountMinSketch { .map_err(insufficient_data(""))?; Family::COUNTMIN.validate_id(family_id)?; - if serial_version != SERIAL_VERSION_1 && serial_version != SERIAL_VERSION_2 { - return Err(Error::deserial(format!( - "unsupported serial version: expected {SERIAL_VERSION_1} or {SERIAL_VERSION_2}, got {serial_version}" - ))); - } + let serial_version = SerialVersion::from_u8(serial_version)?; ensure_preamble_longs_in(&[PREAMBLE_LONGS_SHORT], preamble_longs)?; let num_buckets = cursor .read_u32_le() .map_err(insufficient_data("num_buckets"))?; let num_hashes = cursor.read_u8().map_err(insufficient_data("num_hashes"))?; - if serial_version == SERIAL_VERSION_2 && num_hashes > 127 { - return Err(Error::deserial( - "version 2 num_hashes must be within 1..=127", - )); - } let seed_hash = cursor .read_u16_le() .map_err(insufficient_data("seed_hash"))?; @@ -513,7 +593,8 @@ impl CountMinSketch { ErrorKind::InvalidData, )?; - let entries = entries_for_config_checked(num_hashes, num_buckets)?; + let entries = + serial_version.entries_for_config(num_hashes, num_buckets, ErrorKind::InvalidData)?; let is_empty = (flags & FLAGS_IS_EMPTY) != 0; if !is_empty { let payload_values = entries @@ -573,11 +654,11 @@ impl CountMinSketch { num_buckets: u32, seed: u64, seed_hash: u16, - serial_version: u8, + serial_version: SerialVersion, entries: usize, ) -> Self { let counts = vec![T::ZERO; entries]; - let hash_seeds = make_hash_seeds(seed, num_hashes, serial_version); + let hash_seeds = serial_version.make_hash_seeds(seed, num_hashes); CountMinSketch { num_hashes, buckets: BucketCount::new(num_buckets), @@ -591,20 +672,8 @@ impl CountMinSketch { } fn bucket_index(&self, item: &I, seed: u64) -> usize { - let hash = match self.serial_version { - SERIAL_VERSION_1 => { - let mut hasher = MurmurHash3X64128::with_seed(seed); - item.hash(&mut hasher); - hasher.finish128().0 - } - SERIAL_VERSION_2 => { - let mut hasher = XxHash64::with_seed(seed); - item.hash(&mut hasher); - hasher.finish64() >> 1 - } - _ => unreachable!("unsupported Count-Min serial version"), - }; - self.buckets.index(hash) + self.buckets + .index(self.serial_version.hash_item(item, seed)) } } @@ -665,59 +734,6 @@ impl CountMinSketch { } } -fn entries_for_config(num_hashes: u8, num_buckets: u32) -> Result { - if !(1..=127).contains(&num_hashes) { - return Err(Error::invalid_argument("num_hashes must be within 1..=127")); - } - if num_buckets < 3 { - return Err(Error::invalid_argument("num_buckets must be at least 3")); - } - let entries = (num_hashes as usize) - .checked_mul(num_buckets as usize) - .ok_or_else(|| Error::invalid_argument("num_hashes * num_buckets overflows usize"))?; - if entries >= MAX_TABLE_ENTRIES { - return Err(Error::invalid_argument(format!( - "num_hashes * num_buckets must be < {MAX_TABLE_ENTRIES}" - ))); - } - Ok(entries) -} - -fn entries_for_config_checked(num_hashes: u8, num_buckets: u32) -> Result { - if num_hashes == 0 { - return Err(Error::deserial("num_hashes must be at least 1")); - } - if num_buckets < 3 { - return Err(Error::deserial("num_buckets must be at least 3")); - } - let entries = (num_hashes as usize) - .checked_mul(num_buckets as usize) - .ok_or_else(|| Error::deserial("num_hashes * num_buckets overflows usize"))?; - if entries >= MAX_TABLE_ENTRIES { - return Err(Error::deserial(format!( - "num_hashes * num_buckets must be < {MAX_TABLE_ENTRIES}", - ))); - } - Ok(entries) -} - -fn make_hash_seeds(seed: u64, num_hashes: u8, serial_version: u8) -> Vec { - let mut seeds = Vec::with_capacity(num_hashes as usize); - for i in 0..num_hashes { - let row_seed = match serial_version { - SERIAL_VERSION_1 => { - let mut hasher = MurmurHash3X64128::with_seed(seed); - hasher.write(&u64::from(i).to_le_bytes()); - hasher.finish128().0 - } - SERIAL_VERSION_2 => XxHash64::hash_u64(u64::from(i), seed), - _ => unreachable!("unsupported Count-Min serial version"), - }; - seeds.push(row_seed); - } - seeds -} - #[cfg(test)] mod tests { use super::*; From d10be9125465e6c628571d45e26b4493f0714f5d Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 8 Nov 2026 16:46:00 +0800 Subject: [PATCH 12/13] docs(countmin): clarify cross-language compatibility and restore constructor docs Add a module-level cross-language compatibility note to `countmin`, separate the `CountMinSketch` overview from its serialization and `hash::value` encoding guidance, restore the bulleted `# Errors` list and `# Examples` doctest on `CountMinSketch::with_seed`, and format the `1..=127` row range as inline code in `CHANGELOG.md`. --- CHANGELOG.md | 2 +- datasketches/src/countmin/mod.rs | 3 ++ datasketches/src/countmin/sketch.rs | 58 +++++++++++++++-------------- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ba73c6..9f68d7e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All significant changes to this project will be documented in this file. ### New features -* Use serial version 2 for new `CountMinSketch` instances, enabling cross-language interchange with the documented input encodings. New sketches support 1–127 rows and require a version 2 reader. Existing version 1 data remains readable and updatable, but must be rebuilt from source items to merge with new sketches. +* Use serial version 2 for new `CountMinSketch` instances, enabling cross-language interchange with the documented input encodings. New sketches support `1..=127` rows and require a version 2 reader. Existing version 1 data remains readable and updatable, but must be rebuilt from source items to merge with new sketches. * Add `CpcUnion::serialize`, which produces the same bytes as `to_sketch().serialize()` and can be faster. * Add `HllUnion::serialize` to serialize union results directly. * Add `XorFilter` and `XorFilterBuilder` behind the `xor` feature for immutable membership queries on values or precomputed hashes. Filters support 8-, 16-, and 32-bit fingerprints and can be serialized for later queries. diff --git a/datasketches/src/countmin/mod.rs b/datasketches/src/countmin/mod.rs index 40106225..03c92838 100644 --- a/datasketches/src/countmin/mod.rs +++ b/datasketches/src/countmin/mod.rs @@ -20,6 +20,9 @@ //! The CountMin sketch provides approximate frequency counts for streaming data //! with configurable relative error and confidence bounds. //! +//! Serial version 2 sketches with `i64` counters support cross-language binary interchange when +//! items use compatible hashing; see [`CountMinSketch`] and [`hash::value`](crate::hash::value). +//! //! # Usage //! //! ``` diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index d80f2aaa..f36b2fee 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -44,21 +44,15 @@ const MAX_TABLE_ENTRIES: usize = 1 << 30; /// /// The sketch provides upper and lower bounds on estimated item frequencies /// with configurable relative error and confidence. -/// New sketches use serial version 2. Deserialized version 1 sketches retain their legacy -/// hashing and serialization. Sketches of different versions cannot be merged. -/// Version 1 images are not portable across implementations. /// -/// For version 2 cross-language interchange, use `i64` counters and encode items as follows: -/// * Integers up to 64 bits, including `isize` and `usize`, are widened to 64 bits and hashed as -/// eight little-endian bytes. Signed integers are sign-extended. -/// * Strings use UTF-8 and byte arrays use their contents without prefixes or terminators. Use -/// [`crate::hash::value::raw_bytes`] wrappers. +/// New sketches use serial version 2. Deserialized version 1 sketches retain their legacy +/// hashing and serialization format, and sketches of different versions cannot be merged. /// -/// Version 2 ignores empty encodings: updates have no effect, and estimates and bounds are zero. -/// This includes empty `raw_bytes` wrappers and any [`Hash`] implementation that writes no bytes. -/// Bare strings, slices, and arbitrary custom `Hash` implementations are not automatically -/// portable. Use the same encoding for updates and queries; version 1 data must keep its original -/// encoding. +/// For cross-language version 2 interchange, use `i64` counters and encode inputs consistently: +/// * `i64` and `u64` values are hashed as eight little-endian bytes; use +/// [`crate::hash::value::natural_extend`] for narrower integers. +/// * Use [`crate::hash::value::raw_bytes`] for UTF-8 strings and byte slices, and skip empty inputs +/// when interoperating with implementations that ignore them. #[derive(Debug, Clone, PartialEq)] pub struct CountMinSketch { num_hashes: u8, @@ -199,10 +193,7 @@ impl BucketCount { } impl CountMinSketch { - /// Creates a version 2 CountMin sketch with the default seed. - /// - /// See [`CountMinSketch`] for portable input encodings. Version 2 sketches cannot be merged - /// with version 1 sketches or read by libraries that support only version 1. + /// Creates a new CountMin sketch with the default seed. /// /// # Errors /// @@ -213,25 +204,32 @@ impl CountMinSketch { /// /// ``` /// use datasketches::countmin::CountMinSketch; - /// use datasketches::hash::value::raw_bytes; /// - /// let mut sketch = CountMinSketch::::new(3, 1000)?; - /// sketch.update(raw_bytes::from_str("apple")); - /// assert_eq!(sketch.estimate(raw_bytes::from_str("apple")), 1); - /// # Ok::<(), datasketches::error::Error>(()) + /// let sketch = CountMinSketch::::new(4, 128).unwrap(); + /// assert_eq!(sketch.num_buckets(), 128); /// ``` pub fn new(num_hashes: u8, num_buckets: u32) -> Result { Self::with_seed(num_hashes, num_buckets, DEFAULT_UPDATE_SEED) } - /// Creates a version 2 CountMin sketch with the provided seed. - /// - /// See [`CountMinSketch`] for portable input encodings. + /// Creates a new CountMin sketch with the provided seed. /// /// # Errors /// - /// Returns an error if `num_hashes` is outside `1..=127`, `num_buckets` is less than `3`, the - /// total table size exceeds the supported limit, or the computed seed hash is zero. + /// Returns an error if any of: + /// * `num_hashes` is outside `1..=127`. + /// * `num_buckets` is less than `3`. + /// * The total table size exceeds the supported limit. + /// * The computed seed hash is zero. + /// + /// # Examples + /// + /// ``` + /// use datasketches::countmin::CountMinSketch; + /// + /// let sketch = CountMinSketch::::with_seed(4, 64, 42).unwrap(); + /// assert_eq!(sketch.seed(), 42); + /// ``` pub fn with_seed(num_hashes: u8, num_buckets: u32, seed: u64) -> Result { let entries = SerialVersion::V2.entries_for_config( num_hashes, @@ -322,6 +320,9 @@ impl CountMinSketch { /// Updates the sketch with a single occurrence of the item. /// + /// Use [`hash::value`](crate::hash::value) wrappers to select an input encoding for + /// cross-language hashing. + /// /// # Panics /// /// Panics without modifying the sketch if the total absolute weight would exceed `T::MAX`. @@ -341,7 +342,8 @@ impl CountMinSketch { /// Updates the sketch with the given item and weight. /// - /// Version 2 ignores empty encodings regardless of weight. See [`CountMinSketch`]. + /// Use [`hash::value`](crate::hash::value) wrappers to select an input encoding for + /// cross-language hashing. /// /// # Panics /// From 757454fad50c701995aaf05bb33814a1ff12891a Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 8 Nov 2026 16:48:06 +0800 Subject: [PATCH 13/13] test(countmin): clean up serialization fixtures and legacy compatibility tests Format serialized preamble headers in hexadecimal `u64` words via a shared `words_to_bytes` helper, add `natural_extend` checks to the known-bucket vector test, and remove the redundant 51-word legacy fixture that overlapped existing version 1 round-trip, update, and merge coverage. --- .../tests/serde_tests/countmin.rs | 132 ++++-------------- 1 file changed, 31 insertions(+), 101 deletions(-) diff --git a/tests-integration/tests/serde_tests/countmin.rs b/tests-integration/tests/serde_tests/countmin.rs index f60d1cfe..a0a019d1 100644 --- a/tests-integration/tests/serde_tests/countmin.rs +++ b/tests-integration/tests/serde_tests/countmin.rs @@ -28,10 +28,14 @@ use tests_integration::ZERO_HASH_SEED; use crate::serialization_test_data; +fn words_to_bytes(words: [u64; N]) -> Vec { + words.into_iter().flat_map(u64::to_le_bytes).collect() +} + #[test] fn v2_reads_and_writes_the_published_image() { // TCK discussion #18, section 7.4: update(1, 10), update(9, 810), update("apple", 5). - let words: [u64; 21] = [ + let bytes = words_to_bytes([ 0x0000_0000_0012_0202, 0x0093_cc02_0000_0009, 825, @@ -53,8 +57,7 @@ fn v2_reads_and_writes_the_published_image() { 0, 10, 0, - ]; - let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); + ]); let mut sketch = CountMinSketch::::new(2, 9).unwrap(); let mut empty = bytes[..16].to_vec(); empty[3] = 1; @@ -99,6 +102,7 @@ fn v2_matches_known_bucket_vectors() { // Java XxHash64 vectors from TCK discussion #18, section 7.3. check(-1i64, 9001, [256, 641, 308]); + check(natural_extend::from_i8(-1), 9001, [256, 641, 308]); check(i64::MIN, 9001, [951, 0, 298]); check(raw_bytes::from_str("apple"), 9001, [130, 747, 144]); check( @@ -110,92 +114,11 @@ fn v2_matches_known_bucket_vectors() { // Independently computed with XXH64 over little-endian u64 inputs. check(1u64, 9001, [185, 898, 4]); check(9u64, 9001, [490, 217, 440]); + check(natural_extend::from_u16(9), 9001, [490, 217, 440]); check(1u64, 42, [468, 459, 696]); check(9u64, 42, [841, 428, 322]); } -#[test] -fn legacy_strings_bytes_and_narrow_integers_keep_their_hash_encoding() { - // Generated by Rust v0.6.0: three rows, 16 buckets, default seed. Updates: - // ("apple", 5), (&b"apple"[..], 7), (-1i8, 11), ("", 13), (&b""[..], 17). - let words: [u64; 51] = [ - 1179906, - 41601134833434640, - 53, - 0, - 0, - 0, - 17, - 24, - 0, - 0, - 5, - 0, - 0, - 0, - 7, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 7, - 5, - 17, - 24, - 0, - 0, - 0, - 0, - 0, - 17, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 29, - 0, - 7, - 0, - 0, - 0, - 0, - 0, - ]; - let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); - let mut sketch = CountMinSketch::::deserialize(&bytes).unwrap(); - assert_eq!(sketch.serialize(), bytes); - assert_eq!(sketch.estimate("apple"), 5); - assert_eq!(sketch.estimate(b"apple".as_slice()), 7); - assert_eq!(sketch.estimate(-1i8), 24); - assert_eq!(sketch.estimate(""), 24); - assert_eq!(sketch.estimate(b"".as_slice()), 17); - - sketch.update_with_weight("apple", 2); - sketch.update_with_weight(b"apple".as_slice(), 3); - sketch.update_with_weight("", 4); - sketch.update_with_weight(b"".as_slice(), 5); - assert_eq!(sketch.estimate("apple"), 7); - assert_eq!(sketch.estimate(b"apple".as_slice()), 10); - assert_eq!(sketch.estimate(""), 28); - assert_eq!(sketch.estimate(b"".as_slice()), 22); - assert_eq!(sketch.total_weight(), 67); - assert_eq!(sketch.serialize()[1], 1); - assert_eq!( - CountMinSketch::::deserialize(&sketch.serialize()).unwrap(), - sketch - ); -} - #[test] fn legacy_image_preserves_wrappers_and_custom_hash_inputs() { #[derive(Hash)] @@ -203,9 +126,9 @@ fn legacy_image_preserves_wrappers_and_custom_hash_inputs() { // Generated by Rust v0.6.0: raw_bytes("apple", 5), natural_extend(-1i8, 7), // Key(42, 11), and raw_bytes("", 13), with two rows and nine buckets. - let words: [u64; 21] = [ - 1179906, - 41601130538467337, + let bytes = words_to_bytes([ + 0x0000_0000_0012_0102, + 0x0093_cc02_0000_0009, 36, 7, 0, @@ -225,8 +148,7 @@ fn legacy_image_preserves_wrappers_and_custom_hash_inputs() { 13, 0, 0, - ]; - let bytes: Vec = words.into_iter().flat_map(u64::to_le_bytes).collect(); + ]); let mut sketch = CountMinSketch::::deserialize(&bytes).unwrap(); assert_eq!(sketch.serialize(), bytes); @@ -240,6 +162,7 @@ fn legacy_image_preserves_wrappers_and_custom_hash_inputs() { sketch.update(Key(42)); assert_eq!(sketch.estimate(Key(42)), 23); assert_eq!(sketch.total_weight(), 73); + assert_eq!(sketch.serialize()[1], 1); assert_eq!( CountMinSketch::::deserialize(&sketch.serialize()).unwrap(), sketch @@ -265,27 +188,33 @@ fn v2_row_limit_does_not_reject_legacy_images() { } // Generated by CountMinSketch v0.5.0 with one row, five buckets, seed 123, and key 1 at weight 10. -const RUST_V1_SERIALIZED: [u8; 64] = [ - 2, 1, 18, 0, 0, 0, 0, 0, 5, 0, 0, 0, 1, 182, 5, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 0, 0, 0, 0, - 0, 0, 0, -]; +fn rust_v1_custom_seed_image() -> Vec { + words_to_bytes([ + 0x0000_0000_0012_0102, + 0x0005_b601_0000_0005, + 10, + 0, + 0, + 0, + 0, + 10, + ]) +} #[test] fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { - let mut sketch = - CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + let image = rust_v1_custom_seed_image(); + let mut sketch = CountMinSketch::::deserialize_with_seed(&image, 123).unwrap(); assert_eq!(sketch.total_weight(), 10); assert_eq!(sketch.estimate(1u64), 10); - assert_eq!(sketch.serialize(), RUST_V1_SERIALIZED); + assert_eq!(sketch.serialize(), image); sketch.update_with_weight(1u64, 5); assert_eq!(sketch.total_weight(), 15); assert_eq!(sketch.estimate(1u64), 15); assert_eq!(sketch.serialize()[1], 1); - let mut merged = - CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + let mut merged = CountMinSketch::::deserialize_with_seed(&image, 123).unwrap(); merged.merge(&sketch).unwrap(); assert_eq!(merged.total_weight(), 25); assert_eq!(merged.estimate(1u64), 25); @@ -293,7 +222,8 @@ fn legacy_rust_v1_image_preserves_queries_updates_and_merges() { #[test] fn rejects_merge_across_serial_versions() { - let legacy = CountMinSketch::::deserialize_with_seed(&RUST_V1_SERIALIZED, 123).unwrap(); + let legacy = + CountMinSketch::::deserialize_with_seed(&rust_v1_custom_seed_image(), 123).unwrap(); let mut current = CountMinSketch::::with_seed(1, 5, 123).unwrap(); current.update_with_weight(1u64, 10); let before = current.clone();