From 4b3155baa717e09218943489e519ef1bc3662c94 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:54:59 +0800 Subject: [PATCH 01/37] test(cpc): focus sliding union workload The previous test processed ten million distinct values only to exercise a CPC union after it entered Sliding flavor. Its name and assertions did not state that boundary, so the workload looked arbitrary and dominated the CPC integration-test cost. Use 32 batches of 8,192 values instead and assert the Sliding coupon threshold explicitly. The test still compares the union result with a single sketch over the same stream, while reducing the update count by about 38 times. --- tests-integration/tests/cpc_test/union.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests-integration/tests/cpc_test/union.rs b/tests-integration/tests/cpc_test/union.rs index 54fb1086..533ec2a5 100644 --- a/tests-integration/tests/cpc_test/union.rs +++ b/tests-integration/tests/cpc_test/union.rs @@ -82,13 +82,13 @@ fn test_custom_seed_mismatch() { } #[test] -fn test_large_values() { +fn test_sliding_union_matches_single_sketch() { let mut key = 0; let mut sketch = CpcSketch::new(11).unwrap(); let mut union = CpcUnion::new(11).unwrap(); - for _ in 0..1000 { + for _ in 0..32 { let mut tmp = CpcSketch::new(11).unwrap(); - for _ in 0..10000 { + for _ in 0..8192 { sketch.update(key); tmp.update(key); key += 1; @@ -97,6 +97,7 @@ fn test_large_values() { } let result = union.to_sketch(); assert!(!result.is_empty()); + assert!(result.num_coupons() >= 27 * (1 << 11) / 8); assert_eq!(result.num_coupons(), union.num_coupons()); let estimate = sketch.estimate(); assert_that!( From 09ec0fc96ae1560ea6a8ff1f9a1f5f0a14f97890 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:55:50 +0800 Subject: [PATCH 02/37] fix(tdigest): accept empty split-point queries An empty split-point slice is a valid CDF or PMF request: zero split points define one bin containing the full distribution. The validator instead formed `0..len - 1`, which underflowed before the query could return that bin. Validate ordering with `windows(2)` and check NaNs independently. Mutable and frozen digests now both return `[1.0]` for empty CDF and PMF split points, with a regression covering all four calls. --- CHANGELOG.md | 1 + datasketches/src/tdigest/sketch.rs | 12 ++++-------- tests-integration/tests/tdigest_test/sketch.rs | 13 +++++++++++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dd97ec6..ae8af359 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ All significant changes to this project will be documented in this file. ### Bug fixes +* T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. * Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. * `FrequentItemsSketch` now enforces the cross-language map-size limit of `2^30` consistently. Oversized construction returns `InvalidArgument`, and malformed or oversized serialized images return `InvalidData` instead of panicking or attempting excessive allocation. diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index bd27a4c0..87cff026 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -1021,7 +1021,7 @@ impl TDigest { /// stream given the split points. The value at array position j of the returned CDF array /// is the sum of the returned values in positions 0 through j of the returned PMF array. /// This can be viewed as array of ranks of the given split points plus one more value that - /// is always 1. + /// is always 1. An empty `split_points` slice returns the single value `[1.0]`. /// /// Returns `None` if this t-digest is empty. /// @@ -1059,6 +1059,7 @@ impl TDigest { /// /// An array of m+1 doubles each of which is an approximation to the fraction of the input /// stream values (the mass) that fall into one of those intervals. + /// An empty `split_points` slice returns the single value `[1.0]`. /// /// Returns `None` if this t-digest is empty. /// @@ -1380,15 +1381,10 @@ impl TDigestView<'_> { /// They must be unique, monotonically increasing and not NaN. #[track_caller] fn check_split_points(split_points: &[f64]) { - let len = split_points.len(); - if len == 1 && split_points[0].is_nan() { + if split_points.iter().any(|split_point| split_point.is_nan()) { panic!("split_points must not contain NaN values: {split_points:?}"); } - for i in 0..len - 1 { - if split_points[i] < split_points[i + 1] { - // we must use this positive condition because NaN comparisons are always false - continue; - } + if !split_points.windows(2).all(|pair| pair[0] < pair[1]) { panic!("split_points must be unique and monotonically increasing: {split_points:?}"); } } diff --git a/tests-integration/tests/tdigest_test/sketch.rs b/tests-integration/tests/tdigest_test/sketch.rs index 36732cf2..c0cced92 100644 --- a/tests-integration/tests/tdigest_test/sketch.rs +++ b/tests-integration/tests/tdigest_test/sketch.rs @@ -68,6 +68,19 @@ fn test_one_value() { assert_eq!(tdigest.quantile(1.0), Some(1.0)); } +#[test] +fn test_empty_split_points_define_one_bin() { + let mut tdigest = TDigestMut::new(100).unwrap(); + tdigest.update(1.0); + + assert_eq!(tdigest.cdf(&[]), Some(vec![1.0])); + assert_eq!(tdigest.pmf(&[]), Some(vec![1.0])); + + let tdigest = tdigest.freeze(); + assert_eq!(tdigest.cdf(&[]), Some(vec![1.0])); + assert_eq!(tdigest.pmf(&[]), Some(vec![1.0])); +} + #[test] fn test_maximum_k() { let mut tdigest = TDigestMut::new(u16::MAX).unwrap(); From 507a0bee1aade8b116d5970ef6da2b7441650505 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:56:50 +0800 Subject: [PATCH 03/37] refactor(bloom): return errors from set operations Bloom filters assembled at runtime can legitimately differ in capacity, hash count, or seed. Treating that input mismatch as an assertion made union and intersection unsafe to use at service and storage boundaries. Return `InvalidArgument` before mutating the destination bit array, and return `Ok(())` after a compatible operation. Callers must now handle the `Result`; the integration tests cover both successful set operations and incompatible seeds. --- CHANGELOG.md | 1 + datasketches/src/bloom/mod.rs | 4 +- datasketches/src/bloom/sketch.rs | 40 +++++++++++--------- tests-integration/tests/bloom_test/sketch.rs | 12 +++--- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae8af359..b2c5ceb4 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 +* `BloomFilter::union` and `BloomFilter::intersect` now return `Result`. Callers must handle incompatible filter configurations instead of relying on a panic. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/bloom/mod.rs b/datasketches/src/bloom/mod.rs index 92e5ed82..be16817e 100644 --- a/datasketches/src/bloom/mod.rs +++ b/datasketches/src/bloom/mod.rs @@ -110,12 +110,12 @@ //! filter2.insert("b"); //! //! // Union: recognizes items from either filter -//! filter1.union(&filter2); +//! filter1.union(&filter2).unwrap(); //! assert!(filter1.contains(&"a")); //! assert!(filter1.contains(&"b")); //! //! // Intersect: recognizes only items in both filters -//! // filter1.intersect(&filter2); +//! // filter1.intersect(&filter2).unwrap(); //! //! // Invert: approximately inverts set membership //! // filter1.invert(); diff --git a/datasketches/src/bloom/sketch.rs b/datasketches/src/bloom/sketch.rs index da0c471e..c81a470a 100644 --- a/datasketches/src/bloom/sketch.rs +++ b/datasketches/src/bloom/sketch.rs @@ -159,10 +159,11 @@ impl BloomFilter { /// After merging, this filter will recognize items from either filter /// (plus any false positives from either). /// - /// # Panics + /// # Errors /// - /// Panics if the filters are not compatible (different size, hashes, or seed). - /// Use [`is_compatible()`](Self::is_compatible) to check first. + /// Returns an error if the filters are not compatible (different size, number of hashes, or + /// seed). Use [`is_compatible()`](Self::is_compatible) to check first when an error is not + /// expected. /// /// # Examples /// @@ -181,15 +182,16 @@ impl BloomFilter { /// f1.insert("a"); /// f2.insert("b"); /// - /// f1.union(&f2); + /// f1.union(&f2).unwrap(); /// assert!(f1.contains(&"a")); /// assert!(f1.contains(&"b")); /// ``` - pub fn union(&mut self, other: &BloomFilter) { - assert!( - self.is_compatible(other), - "Cannot union incompatible Bloom filters" - ); + pub fn union(&mut self, other: &BloomFilter) -> Result<(), Error> { + if !self.is_compatible(other) { + return Err(Error::invalid_argument( + "Bloom filters must have matching capacity, number of hashes, and seed", + )); + } // Count bits during union operation (single pass) let mut num_bits_set = 0; @@ -198,6 +200,7 @@ impl BloomFilter { num_bits_set += word.count_ones() as u64; } self.num_bits_set = num_bits_set; + Ok(()) } /// Intersects this filter with another via bitwise AND. @@ -205,9 +208,10 @@ impl BloomFilter { /// After intersection, this filter will recognize only items present in both /// filters (plus false positives). /// - /// # Panics + /// # Errors /// - /// Panics if the filters are not compatible (different size, hashes, or seed). + /// Returns an error if the filters are not compatible (different size, number of hashes, or + /// seed). /// /// # Examples /// @@ -228,15 +232,16 @@ impl BloomFilter { /// f2.insert("b"); /// f2.insert("c"); /// - /// f1.intersect(&f2); + /// f1.intersect(&f2).unwrap(); /// assert!(f1.contains(&"b")); // In both /// // "a" and "c" likely return false now /// ``` - pub fn intersect(&mut self, other: &BloomFilter) { - assert!( - self.is_compatible(other), - "Cannot intersect incompatible Bloom filters" - ); + pub fn intersect(&mut self, other: &BloomFilter) -> Result<(), Error> { + if !self.is_compatible(other) { + return Err(Error::invalid_argument( + "Bloom filters must have matching capacity, number of hashes, and seed", + )); + } // Count bits during intersect operation (single pass) let mut num_bits_set = 0; @@ -245,6 +250,7 @@ impl BloomFilter { num_bits_set += word.count_ones() as u64; } self.num_bits_set = num_bits_set; + Ok(()) } /// Inverts all bits in the filter. diff --git a/tests-integration/tests/bloom_test/sketch.rs b/tests-integration/tests/bloom_test/sketch.rs index 15f44a96..1deefefd 100644 --- a/tests-integration/tests/bloom_test/sketch.rs +++ b/tests-integration/tests/bloom_test/sketch.rs @@ -68,14 +68,14 @@ fn test_union_and_intersection() { let right_bits = right.bits_used(); let mut intersection = left.clone(); - intersection.intersect(&right); + intersection.intersect(&right).unwrap(); assert!(intersection.contains(&"shared")); let intersection_bits = intersection.bits_used(); assert_that!(intersection_bits, le(left_bits)); assert_that!(intersection_bits, le(right_bits)); let mut union = left; - union.union(&right); + union.union(&right).unwrap(); assert!(union.contains(&"shared")); assert!(union.contains(&"left")); assert!(union.contains(&"right")); @@ -122,25 +122,25 @@ fn test_compatibility_checks_all_configuration() { } #[test] -#[should_panic(expected = "Cannot union incompatible Bloom filters")] fn test_union_rejects_incompatible_filters() { let mut left = filter(); let right = BloomFilterBuilder::with_size(NUM_BITS, NUM_HASHES) .seed(SEED + 1) .build() .unwrap(); - left.union(&right); + let error = left.union(&right).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); } #[test] -#[should_panic(expected = "Cannot intersect incompatible Bloom filters")] fn test_intersection_rejects_incompatible_filters() { let mut left = filter(); let right = BloomFilterBuilder::with_size(NUM_BITS, NUM_HASHES) .seed(SEED + 1) .build() .unwrap(); - left.intersect(&right); + let error = left.intersect(&right).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); } #[test] From 60c89974cec708f53d4dd4431421b93e1d2a5fdf Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:57:17 +0800 Subject: [PATCH 04/37] refactor(countmin): return errors from merge Count-Min sketches from different producers may have different hash counts, bucket counts, or seeds. Those are recoverable compatibility errors, not internal invariants that should panic a process. Validate the complete public configuration before changing counters and return `InvalidArgument` on a mismatch. This also removes the self-pointer assertion, which safe Rust cannot reach through simultaneous `&mut self` and `&Self` borrows. Merge examples and tests now handle the `Result`. --- CHANGELOG.md | 1 + datasketches/src/countmin/sketch.rs | 27 ++++++++++--------- .../tests/countmin_test/sketch.rs | 6 ++--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2c5ceb4..5d8673f3 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 * `BloomFilter::union` and `BloomFilter::intersect` now return `Result`. Callers must handle incompatible filter configurations instead of relying on a panic. +* `CountMinSketch::merge` now returns `Result`. Callers must handle incompatible sketch configurations instead of relying on a panic. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 6745f8fe..484e6c4f 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -254,9 +254,9 @@ impl CountMinSketch { /// Merges another sketch into this one. /// - /// # Panics + /// # Errors /// - /// Panics if the sketches have incompatible configurations. + /// Returns an error if the sketches have different numbers of hashes, bucket counts, or seeds. /// /// # Examples /// @@ -269,22 +269,23 @@ impl CountMinSketch { /// left.update("apple"); /// right.update_with_weight("banana", 2); /// - /// left.merge(&right); + /// left.merge(&right).unwrap(); /// assert!(left.estimate("banana") >= 2); /// ``` - pub fn merge(&mut self, other: &CountMinSketch) { - if std::ptr::eq(self, other) { - panic!("Cannot merge a sketch with itself."); + pub fn merge(&mut self, other: &CountMinSketch) -> Result<(), Error> { + if self.num_hashes != other.num_hashes + || self.num_buckets != other.num_buckets + || self.seed != other.seed + { + return Err(Error::invalid_argument( + "Count-Min sketches must have matching numbers of hashes, bucket counts, and seeds", + )); } - assert_eq!(self.num_hashes, other.num_hashes); - assert_eq!(self.num_buckets, other.num_buckets); - assert_eq!(self.seed, other.seed); - assert_eq!(self.counts.len(), other.counts.len()); - let counts_len = self.counts.len(); - for i in 0..counts_len { - self.counts[i] = self.counts[i] + other.counts[i]; + for (count, other_count) in self.counts.iter_mut().zip(&other.counts) { + *count = *count + *other_count; } self.total_weight = self.total_weight + other.total_weight; + Ok(()) } /// Serializes this sketch into the DataSketches CountMin format. diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index 752ec886..d22cf30c 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -218,7 +218,7 @@ fn test_merge() { right.update("a"); right.update("b"); } - left.merge(&right); + left.merge(&right).unwrap(); assert_eq!(left.total_weight(), 18); assert_that!(left.estimate("a"), ge(14)); assert_that!(left.estimate("b"), ge(4)); @@ -272,11 +272,11 @@ fn test_invalid_buckets_return_error() { } #[test] -#[should_panic] fn test_merge_incompatible() { let mut left = CountMinSketch::::new(3, 64).unwrap(); let right = CountMinSketch::::new(2, 64).unwrap(); - left.merge(&right); + let error = left.merge(&right).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); } #[test] From a4647485bc9d5297e74ccb8ad28e3de13ea160e0 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:57:57 +0800 Subject: [PATCH 05/37] refactor(cpc): return errors from union updates A seed mismatch is possible when a union consumes sketches loaded from independent stores or processes. The previous assertion turned that ordinary composition error into a panic. Check the seed before any union state transition, return `InvalidArgument` with the expected and actual seeds, and make every successful flavor path return `Ok(())`. Tests verify the mismatch and update all successful callers to handle the new result. --- CHANGELOG.md | 1 + datasketches/src/cpc/union.rs | 29 +++++++++++++-------- tests-integration/tests/cpc_test/union.rs | 24 ++++++++--------- tests-integration/tests/cpc_test/wrapper.rs | 4 +-- 4 files changed, 33 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d8673f3..0b1a4d89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All significant changes to this project will be documented in this file. * `BloomFilter::union` and `BloomFilter::intersect` now return `Result`. Callers must handle incompatible filter configurations instead of relying on a panic. * `CountMinSketch::merge` now returns `Result`. Callers must handle incompatible sketch configurations instead of relying on a panic. +* `CpcUnion::update` now returns `Result`. Callers must handle seed mismatches instead of relying on a panic. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/cpc/union.rs b/datasketches/src/cpc/union.rs index ebae9d0a..065e1e74 100644 --- a/datasketches/src/cpc/union.rs +++ b/datasketches/src/cpc/union.rs @@ -133,8 +133,8 @@ impl CpcUnion { /// s2.update(&"banana"); /// /// let mut union = CpcUnion::new(12).unwrap(); - /// union.update(&s1); - /// union.update(&s2); + /// union.update(&s1).unwrap(); + /// union.update(&s2).unwrap(); /// /// let result = union.to_sketch(); /// assert_eq!(result.estimate().trunc(), 2.0); @@ -210,15 +210,21 @@ impl CpcUnion { /// Updates this union with a `CpcSketch`. /// - /// # Panics + /// # Errors /// - /// Panics if the seed of the provided sketch does not match the seed of this union. - pub fn update(&mut self, sketch: &CpcSketch) { - assert_eq!(self.seed, sketch.seed()); + /// Returns an error if the seed of the provided sketch does not match the seed of this union. + pub fn update(&mut self, sketch: &CpcSketch) -> Result<(), Error> { + if self.seed != sketch.seed() { + return Err(Error::invalid_argument(format!( + "CPC sketch seed must match union seed: expected {}, got {}", + self.seed, + sketch.seed() + ))); + } let flavor = sketch.flavor(); if flavor == Flavor::Empty { - return; + return Ok(()); } if sketch.lg_k() < self.lg_k { @@ -250,7 +256,7 @@ impl CpcUnion { // are equal. if old_flavor == Flavor::Empty && self.lg_k == sketch.lg_k() { *old_sketch = sketch.clone(); - return; + return Ok(()); } walk_table_updating_sketch(old_sketch, sketch.surprising_value_table()); @@ -263,7 +269,7 @@ impl CpcUnion { self.state = UnionState::BitMatrix(bit_matrix); } - return; + return Ok(()); } // If flavor is past SPARSE mode, the state must have been converted to bitMatrix. @@ -275,7 +281,7 @@ impl CpcUnion { if flavor == Flavor::Sparse { // [Case B] Sparse, bitMatrix valid, accumulator == null or_table_into_matrix(old_matrix, self.lg_k, sketch.surprising_value_table()); - return; + return Ok(()); } if matches!(flavor, Flavor::Hybrid | Flavor::Pinned) { @@ -289,7 +295,7 @@ impl CpcUnion { sketch.lg_k(), ); or_table_into_matrix(old_matrix, self.lg_k, sketch.surprising_value_table()); - return; + return Ok(()); } // [Case D] Sliding, bitMatrix valid, accumulator == null @@ -300,6 +306,7 @@ impl CpcUnion { or_matrix_into_matrix(old_matrix, self.lg_k, &src_matrix, sketch.lg_k()); } } + Ok(()) } fn reduce_k(&mut self, new_lg_k: u8) { diff --git a/tests-integration/tests/cpc_test/union.rs b/tests-integration/tests/cpc_test/union.rs index 533ec2a5..3b38e83e 100644 --- a/tests-integration/tests/cpc_test/union.rs +++ b/tests-integration/tests/cpc_test/union.rs @@ -36,14 +36,14 @@ fn test_two_values() { let mut sketch = CpcSketch::new(11).unwrap(); sketch.update(1); let mut union = CpcUnion::new(11).unwrap(); - union.update(&sketch); + union.update(&sketch).unwrap(); let result = union.to_sketch(); assert!(!result.is_empty()); assert_eq!(result.estimate(), 1.0); sketch.update(2); - union.update(&sketch); + union.update(&sketch).unwrap(); let result = union.to_sketch(); assert!(!result.is_empty()); assert_that!( @@ -60,7 +60,7 @@ fn test_custom_seed() { sketch.update(3); let mut union = CpcUnion::with_seed(11, 123).unwrap(); - union.update(&sketch); + union.update(&sketch).unwrap(); let result = union.to_sketch(); assert!(!result.is_empty()); assert_that!( @@ -70,7 +70,6 @@ fn test_custom_seed() { } #[test] -#[should_panic] fn test_custom_seed_mismatch() { let mut sketch = CpcSketch::with_seed(11, 123).unwrap(); sketch.update(1); @@ -78,7 +77,8 @@ fn test_custom_seed_mismatch() { sketch.update(3); let mut union = CpcUnion::with_seed(11, 234).unwrap(); - union.update(&sketch); + let error = union.update(&sketch).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); } #[test] @@ -93,7 +93,7 @@ fn test_sliding_union_matches_single_sketch() { tmp.update(key); key += 1; } - union.update(&tmp); + union.update(&tmp).unwrap(); } let result = union.to_sketch(); assert!(!result.is_empty()); @@ -113,7 +113,7 @@ fn test_reduce_k_empty() { sketch.update(i); } let mut union = CpcUnion::new(12).unwrap(); - union.update(&sketch); + union.update(&sketch).unwrap(); let result = union.to_sketch(); assert_eq!(result.lg_k(), 11); assert_that!( @@ -130,13 +130,13 @@ fn test_reduce_k_sparse() { for i in 0..100 { sketch12.update(i); } - union.update(&sketch12); + union.update(&sketch12).unwrap(); let mut sketch11 = CpcSketch::new(11).unwrap(); for i in 0..1000 { sketch11.update(i); } - union.update(&sketch11); + union.update(&sketch11).unwrap(); let result = union.to_sketch(); assert_eq!(result.lg_k(), 11); @@ -154,13 +154,13 @@ fn test_reduce_k_window() { for i in 0..500 { sketch12.update(i); } - union.update(&sketch12); + union.update(&sketch12).unwrap(); let mut sketch11 = CpcSketch::new(11).unwrap(); for i in 0..1000 { sketch11.update(i); } - union.update(&sketch11); + union.update(&sketch11).unwrap(); let result = union.to_sketch(); assert_eq!(result.lg_k(), 11); @@ -191,6 +191,6 @@ fn test_union_estimated_size() { for i in 0..1000 { sketch.update(i); } - union.update(&sketch); + union.update(&sketch).unwrap(); assert_eq!(union.estimated_size(), 16496); } diff --git a/tests-integration/tests/cpc_test/wrapper.rs b/tests-integration/tests/cpc_test/wrapper.rs index 2a1367f9..24d118e7 100644 --- a/tests-integration/tests/cpc_test/wrapper.rs +++ b/tests-integration/tests/cpc_test/wrapper.rs @@ -51,8 +51,8 @@ fn test_cpc_wrapper() { assert_that!(concat_wrapper.upper_bound(NumStdDev::Two), eq(dst_ub)); let mut union = CpcUnion::new(lg_k).unwrap(); - union.update(&sk1); - union.update(&sk2); + union.update(&sk1).unwrap(); + union.update(&sk2).unwrap(); let merged = union.to_sketch(); let merged_est = merged.estimate(); let merged_lb = merged.lower_bound(NumStdDev::Two); From 1d5f2e18fd2d6358c54dbb01c9f6fab29031a6ef Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:59:14 +0800 Subject: [PATCH 06/37] refactor(bloom): validate sizing suggestions The public sizing helpers accepted values such as zero items, NaN probabilities, and unsupported bit counts. Floating-point casts and clamping then produced plausible-looking configurations, while the accuracy builder duplicated only part of the validation. Make all three helpers return `Result`, reject invalid domains with `InvalidArgument`, and have `build` delegate to those helpers so direct and builder-based sizing share one contract. The migration is to propagate or unwrap the returned result. --- CHANGELOG.md | 1 + datasketches/src/bloom/sketch.rs | 69 ++++++++++++++------ tests-integration/tests/bloom_test/sketch.rs | 16 +++++ 3 files changed, 65 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b1a4d89..f839e1e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All significant changes to this project will be documented in this file. * `BloomFilter::union` and `BloomFilter::intersect` now return `Result`. Callers must handle incompatible filter configurations instead of relying on a panic. * `CountMinSketch::merge` now returns `Result`. Callers must handle incompatible sketch configurations instead of relying on a panic. * `CpcUnion::update` now returns `Result`. Callers must handle seed mismatches instead of relying on a panic. +* `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp` now return `Result` and reject invalid sizing inputs instead of silently clamping them. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/bloom/sketch.rs b/datasketches/src/bloom/sketch.rs index c81a470a..1fbf7a3b 100644 --- a/datasketches/src/bloom/sketch.rs +++ b/datasketches/src/bloom/sketch.rs @@ -731,16 +731,8 @@ impl BloomFilterBuilder { pub fn build(self) -> Result { let (num_bits, num_hashes) = match self.mode { BloomFilterBuilderMode::Accuracy { max_items, fpp } => { - if max_items == 0 { - return Err(Error::invalid_argument("max_items must be greater than 0")); - } - if !(fpp > 0.0 && fpp <= 1.0) { - return Err(Error::invalid_argument( - "fpp must be between 0.0 and 1.0 (inclusive of 1.0)", - )); - } - let num_bits = Self::suggest_num_bits(max_items, fpp); - let num_hashes = Self::suggest_num_hashes_from_accuracy(max_items, num_bits); + let num_bits = Self::suggest_num_bits(max_items, fpp)?; + let num_hashes = Self::suggest_num_hashes_from_accuracy(max_items, num_bits)?; (num_bits, num_hashes) } BloomFilterBuilderMode::Size { @@ -782,22 +774,33 @@ impl BloomFilterBuilder { /// Formula: `m = -n * ln(p) / (ln(2)^2)` /// where n = max_items, p = fpp /// + /// # Errors + /// + /// Returns an error if `max_items` is zero or `fpp` is outside `(0.0, 1.0]`. + /// /// # Examples /// /// ``` /// use datasketches::bloom::BloomFilterBuilder; /// - /// let bits = BloomFilterBuilder::suggest_num_bits(1000, 0.01); + /// let bits = BloomFilterBuilder::suggest_num_bits(1000, 0.01).unwrap(); /// assert!(bits > 9000 && bits < 10000); // ~9585 bits /// ``` - pub fn suggest_num_bits(max_items: u64, fpp: f64) -> u64 { + pub fn suggest_num_bits(max_items: u64, fpp: f64) -> Result { + if max_items == 0 { + return Err(Error::invalid_argument("max_items must be greater than 0")); + } + if !(fpp > 0.0 && fpp <= 1.0) { + return Err(Error::invalid_argument("fpp must be in (0.0, 1.0]")); + } + let n = max_items as f64; let p = fpp; let ln2_squared = std::f64::consts::LN_2 * std::f64::consts::LN_2; let bits = (-n * p.ln() / ln2_squared).ceil() as u64; - bits.clamp(Self::MIN_NUM_BITS, Self::MAX_NUM_BITS) + Ok(bits.clamp(Self::MIN_NUM_BITS, Self::MAX_NUM_BITS)) } /// Suggests optimal number of hash functions given max items and bit count. @@ -805,24 +808,40 @@ impl BloomFilterBuilder { /// Formula: `k = (m/n) * ln(2)` /// where m = num_bits, n = max_items /// + /// # Errors + /// + /// Returns an error if `max_items` is zero or `num_bits` is outside the supported range. + /// /// # Examples /// /// ``` /// use datasketches::bloom::BloomFilterBuilder; /// - /// let hashes = BloomFilterBuilder::suggest_num_hashes_from_accuracy(1000, 10000); + /// let hashes = BloomFilterBuilder::suggest_num_hashes_from_accuracy(1000, 10000).unwrap(); /// assert_eq!(hashes, 7); // Optimal k ≈ 6.93 /// ``` - pub fn suggest_num_hashes_from_accuracy(max_items: u64, num_bits: u64) -> u16 { + pub fn suggest_num_hashes_from_accuracy(max_items: u64, num_bits: u64) -> Result { + if max_items == 0 { + return Err(Error::invalid_argument("max_items must be greater than 0")); + } + if !(Self::MIN_NUM_BITS..=Self::MAX_NUM_BITS).contains(&num_bits) { + return Err(Error::invalid_argument(format!( + "num_bits must be between {} and {}, got {}", + Self::MIN_NUM_BITS, + Self::MAX_NUM_BITS, + num_bits + ))); + } + let m = num_bits as f64; let n = max_items as f64; // Ceil to avoid selecting too few hashes. let k = (m / n * std::f64::consts::LN_2).ceil(); - k.clamp( + Ok(k.clamp( f64::from(Self::MIN_NUM_HASHES), f64::from(Self::MAX_NUM_HASHES), - ) as u16 + ) as u16) } /// Suggests optimal number of hash functions from target FPP. @@ -830,20 +849,28 @@ impl BloomFilterBuilder { /// Formula: `k = -log2(p)` /// where p = fpp /// + /// # Errors + /// + /// Returns an error if `fpp` is outside `(0.0, 1.0]`. + /// /// # Examples /// /// ``` /// use datasketches::bloom::BloomFilterBuilder; /// - /// let hashes = BloomFilterBuilder::suggest_num_hashes_from_fpp(0.01); + /// let hashes = BloomFilterBuilder::suggest_num_hashes_from_fpp(0.01).unwrap(); /// assert_eq!(hashes, 7); // -log2(0.01) ≈ 6.64 /// ``` - pub fn suggest_num_hashes_from_fpp(fpp: f64) -> u16 { + pub fn suggest_num_hashes_from_fpp(fpp: f64) -> Result { + if !(fpp > 0.0 && fpp <= 1.0) { + return Err(Error::invalid_argument("fpp must be in (0.0, 1.0]")); + } + // Ceil to avoid selecting too few hashes. let k = -fpp.log2(); - k.ceil().clamp( + Ok(k.ceil().clamp( f64::from(Self::MIN_NUM_HASHES), f64::from(Self::MAX_NUM_HASHES), - ) as u16 + ) as u16) } } diff --git a/tests-integration/tests/bloom_test/sketch.rs b/tests-integration/tests/bloom_test/sketch.rs index 1deefefd..92bcfd4a 100644 --- a/tests-integration/tests/bloom_test/sketch.rs +++ b/tests-integration/tests/bloom_test/sketch.rs @@ -176,3 +176,19 @@ fn test_size_builder_rejects_zero_hashes_at_build() { let error = BloomFilterBuilder::with_size(128, 0).build().unwrap_err(); assert_eq!(error.kind(), ErrorKind::InvalidArgument); } + +#[test] +fn test_parameter_suggestions_validate_inputs() { + let errors = [ + BloomFilterBuilder::suggest_num_bits(0, 0.01).unwrap_err(), + BloomFilterBuilder::suggest_num_bits(1000, f64::NAN).unwrap_err(), + BloomFilterBuilder::suggest_num_hashes_from_accuracy(0, 10_000).unwrap_err(), + BloomFilterBuilder::suggest_num_hashes_from_accuracy(1000, 0).unwrap_err(), + BloomFilterBuilder::suggest_num_hashes_from_fpp(0.0).unwrap_err(), + ]; + assert!( + errors + .iter() + .all(|error| error.kind() == ErrorKind::InvalidArgument) + ); +} From 646892ef8989ee288a109207d35a8b84e69ae265 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 20:59:48 +0800 Subject: [PATCH 07/37] refactor(cpc): validate serialized size inputs `max_serialized_bytes` is a public planning helper whose `lg_k` normally comes from caller configuration. Panicking for an out-of-range value made capacity planning less robust than constructing a CPC sketch with the same input. Return `InvalidArgument` outside the supported `[4, 26]` range and preserve the existing size calculation for valid values. Boundary tests cover both sides of the range and a normal configuration. --- CHANGELOG.md | 1 + datasketches/src/cpc/sketch.rs | 20 +++++++++++--------- tests-integration/tests/cpc_test/update.rs | 10 ++++++++++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f839e1e4..b10f44b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All significant changes to this project will be documented in this file. * `CountMinSketch::merge` now returns `Result`. Callers must handle incompatible sketch configurations instead of relying on a panic. * `CpcUnion::update` now returns `Result`. Callers must handle seed mismatches instead of relying on a panic. * `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp` now return `Result` and reject invalid sizing inputs instead of silently clamping them. +* `CpcSketch::max_serialized_bytes` now returns `Result` and reports an invalid `lg_k` instead of panicking. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/cpc/sketch.rs b/datasketches/src/cpc/sketch.rs index 8ebaeb42..0a6ce00f 100644 --- a/datasketches/src/cpc/sketch.rs +++ b/datasketches/src/cpc/sketch.rs @@ -880,14 +880,15 @@ impl CpcSketch { /// /// For small values of `n` the size can be much smaller. /// - /// # Panics + /// # Errors /// - /// Panics if `lg_k` is not in the range `[4, 26]`. - pub fn max_serialized_bytes(lg_k: u8) -> usize { - assert!( - (MIN_LG_K..=MAX_LG_K).contains(&lg_k), - "lg_k out of range; got {lg_k}", - ); + /// Returns an error if `lg_k` is not in the range `[4, 26]`. + pub fn max_serialized_bytes(lg_k: u8) -> Result { + if !(MIN_LG_K..=MAX_LG_K).contains(&lg_k) { + return Err(Error::invalid_argument(format!( + "lg_k must be in [{MIN_LG_K}, {MAX_LG_K}], got {lg_k}" + ))); + } // These empirical values for the 99.9th percentile of size in bytes were measured using // 100,000 trials. The value for each trial is the maximum of 5*16=80 measurements @@ -916,12 +917,13 @@ impl CpcSketch { 314656, // lg_k = 19 ]; - if lg_k <= EMPIRICAL_SIZE_MAX_LGK { + let max_bytes = if lg_k <= EMPIRICAL_SIZE_MAX_LGK { EMPIRICAL_MAX_SIZE_BYTES[(lg_k - MIN_LG_K) as usize] + MAX_PREAMBLE_SIZE_BYTES } else { let k = 1 << lg_k; ((EMPIRICAL_MAX_SIZE_FACTOR * k as f64) as usize) + MAX_PREAMBLE_SIZE_BYTES - } + }; + Ok(max_bytes) } } diff --git a/tests-integration/tests/cpc_test/update.rs b/tests-integration/tests/cpc_test/update.rs index e7a22ebf..ab806f6e 100644 --- a/tests-integration/tests/cpc_test/update.rs +++ b/tests-integration/tests/cpc_test/update.rs @@ -17,6 +17,7 @@ use datasketches::common::NumStdDev; use datasketches::cpc::CpcSketch; +use datasketches::error::ErrorKind; use googletest::assert_that; use googletest::prelude::ge; use googletest::prelude::le; @@ -60,3 +61,12 @@ fn test_many_values() { assert_that!(sketch.estimate(), le(sketch.upper_bound(NumStdDev::One))); assert!(sketch.validate()); } + +#[test] +fn test_max_serialized_bytes_validates_lg_k() { + assert!(CpcSketch::max_serialized_bytes(11).unwrap() > 0); + for lg_k in [3, 27] { + let error = CpcSketch::max_serialized_bytes(lg_k).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } +} From 090f3f6050c834a81bdbfffc7906b89f4fec1616 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:00:37 +0800 Subject: [PATCH 08/37] fix(frequencies): reject undersized maps `FrequentItemsSketch::new` accepted power-of-two map sizes below eight and silently promoted them inside the private constructor. A caller asking for one, two, or four entries therefore received a differently configured sketch without an error. Reject every map size below the algorithm minimum at the public boundary. One table-driven regression replaces the duplicate item-type tests because map-size validation is independent of the stored key type. --- CHANGELOG.md | 1 + datasketches/src/frequencies/sketch.rs | 10 ++++++++-- tests-integration/tests/frequencies_test/update.rs | 14 +++++--------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b10f44b3..4e20e2e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All significant changes to this project will be documented in this file. * `CpcUnion::update` now returns `Result`. Callers must handle seed mismatches instead of relying on a panic. * `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp` now return `Result` and reject invalid sizing inputs instead of silently clamping them. * `CpcSketch::max_serialized_bytes` now returns `Result` and reports an invalid `lg_k` instead of panicking. +* `FrequentItemsSketch::new` now rejects map sizes below the minimum of 8 instead of silently rounding them up. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/frequencies/sketch.rs b/datasketches/src/frequencies/sketch.rs index 0f9c4b08..e3c5272a 100644 --- a/datasketches/src/frequencies/sketch.rs +++ b/datasketches/src/frequencies/sketch.rs @@ -39,6 +39,7 @@ type SerializeItem = fn(&mut SketchBytes, &T); type DeserializeItems = fn(SketchSlice<'_>, usize) -> Result, Error>; const LG_MIN_MAP_SIZE: u8 = 3; +const MIN_MAP_SIZE: usize = 1usize << LG_MIN_MAP_SIZE; // Java represents map sizes as positive `int` powers of two, while the C++ // implementation uses 32-bit table indices. Keep Rust configurations within // the same cross-language range. @@ -137,8 +138,8 @@ impl FrequentItemsSketch { /// /// # Errors /// - /// Returns an error if `max_map_size` is not a power of two or exceeds `2^30`, the maximum - /// supported by the cross-language format implementations. + /// Returns an error if `max_map_size` is not a power of two in the range `[8, 2^30]`. The upper + /// bound is the maximum supported by the cross-language format implementations. /// /// # Examples /// @@ -154,6 +155,11 @@ impl FrequentItemsSketch { if !max_map_size.is_power_of_two() { return Err(Error::invalid_argument("max_map_size must be a power of 2")); } + if max_map_size < MIN_MAP_SIZE { + return Err(Error::invalid_argument(format!( + "max_map_size must be at least {MIN_MAP_SIZE}" + ))); + } if max_map_size > MAX_MAP_SIZE { return Err(Error::invalid_argument(format!( "max_map_size must not exceed {MAX_MAP_SIZE}" diff --git a/tests-integration/tests/frequencies_test/update.rs b/tests-integration/tests/frequencies_test/update.rs index 09dbe48d..b8e2d26e 100644 --- a/tests-integration/tests/frequencies_test/update.rs +++ b/tests-integration/tests/frequencies_test/update.rs @@ -551,15 +551,11 @@ fn test_longs_reset() { } #[test] -fn test_longs_invalid_map_size_returns_error() { - let error = FrequentItemsSketch::::new(6).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); -} - -#[test] -fn test_items_invalid_map_size_returns_error() { - let error = FrequentItemsSketch::::new(6).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); +fn test_invalid_map_size_returns_error() { + for max_map_size in [1, 2, 4, 6] { + let error = FrequentItemsSketch::::new(max_map_size).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } } #[test] From 2a2f0fd4bf4dba32e56bcb1573e4265d88aec498 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:01:52 +0800 Subject: [PATCH 09/37] refactor(frequencies): use map sizes in error helpers The constructor accepts an actual maximum map size, but the public error helpers accepted its base-two logarithm. That unit mismatch was easy to misuse, exposed an implementation detail, and allowed unchecked shift inputs; `apriori_error` also used a signed weight although stream weights are unsigned. Replace `epsilon_for_lg` with fallible `epsilon_for_max_map_size`, make `apriori_error` accept the same map-size unit and a `u64` weight, and expose `max_map_size` for round-tripping configuration. Constructor and helpers now share one validator, with valid and invalid examples covered. --- CHANGELOG.md | 1 + datasketches/src/frequencies/sketch.rs | 82 ++++++++++++++----- .../tests/frequencies_test/update.rs | 11 ++- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e20e2e3..7666102d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All significant changes to this project will be documented in this file. * `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp` now return `Result` and reject invalid sizing inputs instead of silently clamping them. * `CpcSketch::max_serialized_bytes` now returns `Result` and reports an invalid `lg_k` instead of panicking. * `FrequentItemsSketch::new` now rejects map sizes below the minimum of 8 instead of silently rounding them up. +* Replace `FrequentItemsSketch::epsilon_for_lg` with the fallible `epsilon_for_max_map_size`, and change `apriori_error` to accept the same maximum map size plus an unsigned stream weight. These helpers now match the constructor's units, and `max_map_size` exposes the configured value. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/frequencies/sketch.rs b/datasketches/src/frequencies/sketch.rs index e3c5272a..d8c765bb 100644 --- a/datasketches/src/frequencies/sketch.rs +++ b/datasketches/src/frequencies/sketch.rs @@ -55,6 +55,28 @@ fn map_capacity_for_lg(lg_map_size: u8) -> usize { (1usize << lg_map_size) * LOAD_FACTOR_NUMERATOR / LOAD_FACTOR_DENOMINATOR } +fn lg_for_max_map_size(max_map_size: usize) -> Result { + if !max_map_size.is_power_of_two() { + return Err(Error::invalid_argument("max_map_size must be a power of 2")); + } + if max_map_size < MIN_MAP_SIZE { + return Err(Error::invalid_argument(format!( + "max_map_size must be at least {MIN_MAP_SIZE}" + ))); + } + if max_map_size > MAX_MAP_SIZE { + return Err(Error::invalid_argument(format!( + "max_map_size must not exceed {MAX_MAP_SIZE}" + ))); + } + Ok(max_map_size.trailing_zeros() as u8) +} + +fn epsilon_for_lg(lg_max_map_size: u8) -> f64 { + debug_assert!((LG_MIN_MAP_SIZE..=LG_MAX_MAP_SIZE).contains(&lg_max_map_size)); + EPSILON_FACTOR / (1u64 << lg_max_map_size) as f64 +} + fn validate_lg_map_sizes(lg_max: u8, lg_cur: u8) -> Result<(), Error> { if lg_cur < LG_MIN_MAP_SIZE { return Err(Error::deserial(format!( @@ -152,20 +174,7 @@ impl FrequentItemsSketch { /// assert_eq!(sketch.num_active_items(), 2); /// ``` pub fn new(max_map_size: usize) -> Result { - if !max_map_size.is_power_of_two() { - return Err(Error::invalid_argument("max_map_size must be a power of 2")); - } - if max_map_size < MIN_MAP_SIZE { - return Err(Error::invalid_argument(format!( - "max_map_size must be at least {MIN_MAP_SIZE}" - ))); - } - if max_map_size > MAX_MAP_SIZE { - return Err(Error::invalid_argument(format!( - "max_map_size must not exceed {MAX_MAP_SIZE}" - ))); - } - let lg_max_map_size = max_map_size.trailing_zeros() as u8; + let lg_max_map_size = lg_for_max_map_size(max_map_size)?; Ok(Self::with_lg_map_sizes(lg_max_map_size, LG_MIN_MAP_SIZE)) } @@ -251,17 +260,43 @@ impl FrequentItemsSketch { /// Returns the epsilon error parameter for this sketch. pub fn epsilon(&self) -> f64 { - Self::epsilon_for_lg(self.lg_max_map_size) + epsilon_for_lg(self.lg_max_map_size) } - /// Returns the epsilon error parameter for the given `lg_max_map_size`. - pub fn epsilon_for_lg(lg_max_map_size: u8) -> f64 { - EPSILON_FACTOR / (1u64 << lg_max_map_size) as f64 + /// Returns the epsilon error parameter for the given maximum map size. + /// + /// # Errors + /// + /// Returns an error if `max_map_size` is not a power of two in the range `[8, 2^30]`. + /// + /// # Examples + /// + /// ``` + /// use datasketches::frequencies::FrequentItemsSketch; + /// + /// let epsilon = FrequentItemsSketch::::epsilon_for_max_map_size(1024).unwrap(); + /// assert_eq!(epsilon, 3.5 / 1024.0); + /// ``` + pub fn epsilon_for_max_map_size(max_map_size: usize) -> Result { + Ok(epsilon_for_lg(lg_for_max_map_size(max_map_size)?)) } - /// Returns the a priori error estimate. - pub fn apriori_error(lg_max_map_size: u8, estimated_total_weight: i64) -> f64 { - Self::epsilon_for_lg(lg_max_map_size) * estimated_total_weight as f64 + /// Returns the a priori error estimate for a maximum map size and estimated total weight. + /// + /// # Errors + /// + /// Returns an error if `max_map_size` is not a power of two in the range `[8, 2^30]`. + /// + /// # Examples + /// + /// ``` + /// use datasketches::frequencies::FrequentItemsSketch; + /// + /// let error = FrequentItemsSketch::::apriori_error(1024, 10_000).unwrap(); + /// assert_eq!(error, 3.5 / 1024.0 * 10_000.0); + /// ``` + pub fn apriori_error(max_map_size: usize, estimated_total_weight: u64) -> Result { + Ok(Self::epsilon_for_max_map_size(max_map_size)? * estimated_total_weight as f64) } /// Returns the maximum map capacity for this sketch. @@ -278,6 +313,11 @@ impl FrequentItemsSketch { self.cur_map_cap } + /// Returns the configured maximum map size. + pub fn max_map_size(&self) -> usize { + 1usize << self.lg_max_map_size + } + /// Returns the configured `lg_max_map_size`. pub fn lg_max_map_size(&self) -> u8 { self.lg_max_map_size diff --git a/tests-integration/tests/frequencies_test/update.rs b/tests-integration/tests/frequencies_test/update.rs index b8e2d26e..7eb16fb6 100644 --- a/tests-integration/tests/frequencies_test/update.rs +++ b/tests-integration/tests/frequencies_test/update.rs @@ -69,20 +69,27 @@ fn test_capacity_and_epsilon_helpers() { let longs: FrequentItemsSketch = FrequentItemsSketch::new(8).unwrap(); assert_eq!(longs.current_map_capacity(), 6); assert_eq!(longs.maximum_map_capacity(), 6); + assert_eq!(longs.max_map_size(), 8); assert_eq!(longs.lg_cur_map_size(), 3); assert_eq!(longs.lg_max_map_size(), 3); - let epsilon = FrequentItemsSketch::::epsilon_for_lg(10); + let epsilon = FrequentItemsSketch::::epsilon_for_max_map_size(1024).unwrap(); let expected = 3.5 / 1024.0; assert_that!(epsilon, near(expected, 1e-12)); - let apriori = FrequentItemsSketch::::apriori_error(10, 10_000); + let apriori = FrequentItemsSketch::::apriori_error(1024, 10_000).unwrap(); assert_that!(apriori, near(expected * 10_000.0, 1e-9)); + let invalid_epsilon = FrequentItemsSketch::::epsilon_for_max_map_size(6).unwrap_err(); + assert_eq!(invalid_epsilon.kind(), ErrorKind::InvalidArgument); + let invalid_apriori = FrequentItemsSketch::::apriori_error(4, 10_000).unwrap_err(); + assert_eq!(invalid_apriori.kind(), ErrorKind::InvalidArgument); + let items: FrequentItemsSketch = FrequentItemsSketch::new(1024).unwrap(); assert_that!(items.epsilon(), near(expected, 1e-12)); assert_eq!(items.current_map_capacity(), 6); assert_eq!(items.maximum_map_capacity(), 768); + assert_eq!(items.max_map_size(), 1024); assert_eq!(items.lg_max_map_size(), 10); } From d04d58611c9fb7e86ee444323204b3083f0608a2 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:03:09 +0800 Subject: [PATCH 10/37] refactor(tdigest): replace deserialize precision flag A call such as `deserialize(bytes, false)` does not reveal which wire format is being selected. The DataSketches C++ float image does not encode its scalar width, so automatic detection cannot remove that choice, but a boolean makes it too easy to reverse. Make the standard double-precision path `deserialize(bytes)` and add the explicit `deserialize_f32(bytes)` entry point for C++ `tdigest` images. Both delegate to one private decoder; reference-format auto-detection remains unchanged, and fixtures plus benchmarks exercise the named paths. --- CHANGELOG.md | 1 + benchmarks/tdigest/merge.rs | 4 +-- benchmarks/tdigest/serde.rs | 4 +-- datasketches/src/tdigest/sketch.rs | 31 ++++++++++++------- .../tests/serde_tests/tdigest.rs | 31 +++++++++++-------- 5 files changed, 43 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7666102d..25c7b220 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All significant changes to this project will be documented in this file. * `CpcSketch::max_serialized_bytes` now returns `Result` and reports an invalid `lg_k` instead of panicking. * `FrequentItemsSketch::new` now rejects map sizes below the minimum of 8 instead of silently rounding them up. * Replace `FrequentItemsSketch::epsilon_for_lg` with the fallible `epsilon_for_max_map_size`, and change `apriori_error` to accept the same maximum map size plus an unsigned stream weight. These helpers now match the constructor's units, and `max_map_size` exposes the configured value. +* Replace the `is_f32` flag on `TDigestMut::deserialize` with separate `deserialize` and `deserialize_f32` entry points, making the serialized precision explicit at the call site. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/benchmarks/tdigest/merge.rs b/benchmarks/tdigest/merge.rs index 306c3172..13559c66 100644 --- a/benchmarks/tdigest/merge.rs +++ b/benchmarks/tdigest/merge.rs @@ -111,7 +111,7 @@ fn serialized_partials(bencher: Bencher, rows_per_partial: usize) { .bench_local(|| { let mut merged = TDigestMut::default(); for partial in black_box(&partials) { - let partial = TDigestMut::deserialize(partial, false).unwrap(); + let partial = TDigestMut::deserialize(partial).unwrap(); merged.merge(&partial); } black_box(merged.quantile(0.5)) @@ -131,7 +131,7 @@ fn serialized_overlapping_partials(bencher: Bencher) { .bench_local(|| { let mut merged = TDigestMut::default(); for partial in black_box(&partials) { - let partial = TDigestMut::deserialize(partial, false).unwrap(); + let partial = TDigestMut::deserialize(partial).unwrap(); merged.merge(&partial); } black_box(merged.quantile(0.5)) diff --git a/benchmarks/tdigest/serde.rs b/benchmarks/tdigest/serde.rs index 85688150..7d5e0a82 100644 --- a/benchmarks/tdigest/serde.rs +++ b/benchmarks/tdigest/serde.rs @@ -100,7 +100,7 @@ fn deserialize_small_partial_groups(bencher: Bencher) { .bench_local(|| { let digests = bytes .iter() - .map(|bytes| TDigestMut::deserialize(bytes, false).unwrap()) + .map(|bytes| TDigestMut::deserialize(bytes).unwrap()) .collect::>(); black_box(digests) }); @@ -131,7 +131,7 @@ fn deserialize_partial_groups(bencher: Bencher) { .bench_local(|| { let digests = bytes .iter() - .map(|bytes| TDigestMut::deserialize(bytes, false).unwrap()) + .map(|bytes| TDigestMut::deserialize(bytes).unwrap()) .collect::>(); black_box(digests) }); diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index 87cff026..6dbf3924 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -498,7 +498,7 @@ impl TDigestMut { /// let mut sketch = TDigestMut::new(100).unwrap(); /// sketch.update(1.0); /// let bytes = sketch.serialize(); - /// let decoded = TDigestMut::deserialize(&bytes, false).unwrap(); + /// let decoded = TDigestMut::deserialize(&bytes).unwrap(); /// assert_eq!(decoded.max_value(), Some(1.0)); /// ``` pub fn serialize(&mut self) -> Vec { @@ -574,15 +574,11 @@ impl TDigestMut { bytes.into_bytes() } - /// Deserializes a mutable t-digest from bytes. + /// Deserializes a mutable t-digest from the standard double-precision format. /// - /// Supports reading compact format with (float, int) centroids as opposed to (double, long) to - /// represent (mean, weight). [^1] - /// - /// Supports reading format of the reference implementation (auto-detected) [^2]. - /// - /// [^1]: This is to support reading the `tdigest` format from the C++ implementation. - /// [^2]: + /// The format of the [reference implementation](https://github.com/tdunning/t-digest) is + /// auto-detected. Use [`deserialize_f32()`](Self::deserialize_f32) for the compact + /// DataSketches C++ `tdigest` format. /// /// # Examples /// @@ -593,10 +589,23 @@ impl TDigestMut { /// sketch.update(1.0); /// sketch.update(2.0); /// let bytes = sketch.serialize(); - /// let decoded = TDigestMut::deserialize(&bytes, false).unwrap(); + /// let decoded = TDigestMut::deserialize(&bytes).unwrap(); /// assert_eq!(decoded.max_value(), Some(2.0)); /// ``` - pub fn deserialize(bytes: &[u8], is_f32: bool) -> Result { + pub fn deserialize(bytes: &[u8]) -> Result { + Self::deserialize_impl(bytes, false) + } + + /// Deserializes a mutable t-digest from the compact single-precision DataSketches format. + /// + /// This format stores centroid means and weights as `(f32, u32)` and is emitted by the C++ + /// `tdigest` implementation. Its header does not identify the scalar width, so callers + /// must select this entry point explicitly. + pub fn deserialize_f32(bytes: &[u8]) -> Result { + Self::deserialize_impl(bytes, true) + } + + fn deserialize_impl(bytes: &[u8], is_f32: bool) -> Result { let mut cursor = SketchSlice::new(bytes); let preamble_longs = cursor diff --git a/tests-integration/tests/serde_tests/tdigest.rs b/tests-integration/tests/serde_tests/tdigest.rs index 63b0eb0b..1011df2f 100644 --- a/tests-integration/tests/serde_tests/tdigest.rs +++ b/tests-integration/tests/serde_tests/tdigest.rs @@ -46,7 +46,12 @@ fn patterned_digest(k: u16, len: usize, salt: usize) -> TDigestMut { fn test_sketch_file(path: PathBuf, n: u64, with_buffer: bool, is_f32: bool) { let bytes = fs::read(&path).unwrap(); - let td = TDigestMut::deserialize(&bytes, is_f32).unwrap(); + let td = if is_f32 { + TDigestMut::deserialize_f32(&bytes) + } else { + TDigestMut::deserialize(&bytes) + } + .unwrap(); let td = td.freeze(); let path = path.display(); @@ -111,7 +116,7 @@ fn test_deserialize_from_reference_implementation() { ] { let path = serialization_test_data("reference_files", filename); let bytes = fs::read(&path).unwrap(); - let td = TDigestMut::deserialize(&bytes, false).unwrap(); + let td = TDigestMut::deserialize(&bytes).unwrap(); let td = td.freeze(); let n = 10000; @@ -174,7 +179,7 @@ fn test_empty() { assert_eq!(bytes.len(), 8); let td = td.freeze(); - let deserialized_td = TDigestMut::deserialize(&bytes, false).unwrap(); + let deserialized_td = TDigestMut::deserialize(&bytes).unwrap(); let deserialized_td = deserialized_td.freeze(); assert_eq!(td.k(), deserialized_td.k()); assert_eq!(td.total_weight(), deserialized_td.total_weight()); @@ -190,7 +195,7 @@ fn test_single_value() { let bytes = td.serialize(); assert_eq!(bytes.len(), 16); - let deserialized_td = TDigestMut::deserialize(&bytes, false).unwrap(); + let deserialized_td = TDigestMut::deserialize(&bytes).unwrap(); let deserialized_td = deserialized_td.freeze(); assert_eq!(deserialized_td.k(), 200); assert_eq!(deserialized_td.total_weight(), 1); @@ -210,7 +215,7 @@ fn test_many_values() { assert_eq!(bytes.len(), 1584); let td = td.freeze(); - let deserialized_td = TDigestMut::deserialize(&bytes, false).unwrap(); + let deserialized_td = TDigestMut::deserialize(&bytes).unwrap(); let deserialized_td = deserialized_td.freeze(); assert_eq!(td.k(), deserialized_td.k()); assert_eq!(td.total_weight(), deserialized_td.total_weight()); @@ -252,10 +257,10 @@ fn test_serialized_bytes_stable_for_full_and_merged_digests() { let mut left = patterned_digest(10, 199, 2); let left = left.serialize(); - let mut left = TDigestMut::deserialize(&left, false).unwrap(); + let mut left = TDigestMut::deserialize(&left).unwrap(); let mut right = patterned_digest(10, 199, 3); let right = right.serialize(); - let right = TDigestMut::deserialize(&right, false).unwrap(); + let right = TDigestMut::deserialize(&right).unwrap(); left.merge(&right); let bytes = left.serialize(); assert_eq!(bytes.len(), 272); @@ -276,7 +281,7 @@ fn test_updates_normalize_overfull_deserialized_buffer_without_centroids() { bytes.extend_from_slice(&10_f64.to_le_bytes()); } - let mut tdigest = TDigestMut::deserialize(&bytes, false).unwrap(); + let mut tdigest = TDigestMut::deserialize(&bytes).unwrap(); for _ in 0..10_000 { tdigest.update(10.0); } @@ -290,7 +295,7 @@ fn test_updates_normalize_overfull_deserialized_buffer_without_centroids() { let serialized = tdigest.serialize(); assert_eq!(&serialized[12..16], &0_u32.to_le_bytes()); - let roundtrip = TDigestMut::deserialize(&serialized, false).unwrap(); + let roundtrip = TDigestMut::deserialize(&serialized).unwrap(); assert_eq!(roundtrip.total_weight(), 10_841); assert_eq!(roundtrip.min_value(), Some(1.0)); assert_eq!(roundtrip.max_value(), Some(10.0)); @@ -309,7 +314,7 @@ fn test_updates_normalize_overfull_deserialized_mixed_buffer() { bytes.extend_from_slice(&1_000_f64.to_le_bytes()); } - let mut tdigest = TDigestMut::deserialize(&bytes, false).unwrap(); + let mut tdigest = TDigestMut::deserialize(&bytes).unwrap(); for _ in 0..10_000 { tdigest.update(1_000.0); } @@ -323,7 +328,7 @@ fn test_updates_normalize_overfull_deserialized_mixed_buffer() { let serialized = tdigest.serialize(); assert_eq!(&serialized[12..16], &0_u32.to_le_bytes()); - let roundtrip = TDigestMut::deserialize(&serialized, false).unwrap(); + let roundtrip = TDigestMut::deserialize(&serialized).unwrap(); assert_eq!(roundtrip.total_weight(), 11_681); assert_eq!(roundtrip.min_value(), Some(1.0)); assert_eq!(roundtrip.max_value(), Some(1_000.0)); @@ -338,7 +343,7 @@ fn test_deserialize_rejects_truncated_large_payload_before_allocation() { bytes[8..12].copy_from_slice(&u32::MAX.to_le_bytes()); bytes[12..16].copy_from_slice(&u32::MAX.to_le_bytes()); - assert!(TDigestMut::deserialize(&bytes, false).is_err()); + assert!(TDigestMut::deserialize(&bytes).is_err()); } #[test] @@ -354,7 +359,7 @@ fn test_large_weights_produce_finite_extreme_quantile() { bytes[40..48].copy_from_slice(&((1_u64 << 52) - 1).to_le_bytes()); bytes[56..64].copy_from_slice(&(1_u64 << 52).to_le_bytes()); - let mut tdigest = TDigestMut::deserialize(&bytes, false).unwrap(); + let mut tdigest = TDigestMut::deserialize(&bytes).unwrap(); let quantile = tdigest.quantile(0.25).unwrap(); assert_that!(quantile, all!(is_finite(), ge(lower), le(f64::MAX))); } From c5586483d89952080a2f7adb57be9b7190a84b2f Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:04:48 +0800 Subject: [PATCH 11/37] feat(tdigest): persist frozen digests directly The immutable `TDigest` is the natural query and sharing form, but persistence required callers to convert through `TDigestMut`. That added ownership churn and made the two public forms unnecessarily asymmetric. Extract the existing compressed-image writer into a shared helper, then add `serialize`, `deserialize`, and `deserialize_f32` to `TDigest`. Mutable serialization still compresses first, immutable deserialization still passes through the validated mutable decoder, and the existing byte-stability snapshots ensure the wire format does not change. --- CHANGELOG.md | 1 + datasketches/src/tdigest/sketch.rs | 186 +++++++++++------- .../tests/serde_tests/tdigest.rs | 16 ++ 3 files changed, 130 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25c7b220..61e64b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ All significant changes to this project will be documented in this file. ### New features +* `TDigest` can now be serialized and deserialized directly without converting through `TDigestMut` at the call site. * Add Relative Error Quantiles (REQ) sketches behind the `req` feature, including configurable high- or low-rank accuracy, rank, quantile, PMF, and CDF queries, merging, totally ordered custom item types, the `ReqFloat` adapter for non-NaN floating-point values, and C++/Java-compatible serialization. ### Performance improvements diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index 6dbf3924..613c2c7d 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -503,75 +503,14 @@ impl TDigestMut { /// ``` pub fn serialize(&mut self) -> Vec { self.compress(); - let centroids = self.buffer.compressed_centroids(); - - let mut total_size = 0; - if self.is_empty() || self.is_single_value() { - // 1 byte preamble - // + 1 byte serial version - // + 1 byte family - // + 2 bytes k - // + 1 byte flags - // + 2 bytes unused - total_size += size_of::(); - } else { - // all of the above - // + 4 bytes num centroids - // + 4 bytes num buffered - total_size += size_of::() * 2; - } - if self.is_empty() { - // nothing more - } else if self.is_single_value() { - // + 8 bytes single value - total_size += size_of::(); - } else { - // + 8 bytes min - // + 8 bytes max - total_size += size_of::() * 2; - // + (8+8) bytes per centroid - total_size += centroids.len() * (size_of::() + size_of::()); - } - - let mut bytes = SketchBytes::with_capacity(total_size); - bytes.write_u8(match self.total_weight() { - 0 => PREAMBLE_LONGS_EMPTY_OR_SINGLE, - 1 => PREAMBLE_LONGS_EMPTY_OR_SINGLE, - _ => PREAMBLE_LONGS_MULTIPLE, - }); - bytes.write_u8(SERIAL_VERSION); - bytes.write_u8(Family::TDIGEST.id); - bytes.write_u16_le(self.k); - bytes.write_u8({ - let mut flags = 0; - if self.is_empty() { - flags |= FLAGS_IS_EMPTY; - } - if self.is_single_value() { - flags |= FLAGS_IS_SINGLE_VALUE; - } - if self.reverse_merge { - flags |= FLAGS_REVERSE_MERGE; - } - flags - }); - bytes.write_u16_le(0); // unused - if self.is_empty() { - return bytes.into_bytes(); - } - if self.is_single_value() { - bytes.write_f64_le(self.min); - return bytes.into_bytes(); - } - bytes.write_u32_le(centroids.len() as u32); - bytes.write_u32_le(0); // unused - bytes.write_f64_le(self.min); - bytes.write_f64_le(self.max); - for centroid in centroids { - bytes.write_f64_le(centroid.mean); - bytes.write_u64_le(centroid.weight.get()); - } - bytes.into_bytes() + serialize_compressed( + self.k, + self.reverse_merge, + self.min, + self.max, + self.buffer.compressed_centroids(), + self.compressed_weight, + ) } /// Deserializes a mutable t-digest from the standard double-precision format. @@ -862,10 +801,6 @@ impl TDigestMut { } } - fn is_single_value(&self) -> bool { - self.total_weight() == 1 - } - /// Processes unmerged values and merges centroids if needed. fn compress(&mut self) { let additional_weight = self.buffer.unmerged_len() as u64; @@ -959,6 +894,71 @@ impl TDigestMut { } } +fn serialize_compressed( + k: u16, + reverse_merge: bool, + min: f64, + max: f64, + centroids: &[Centroid], + total_weight: u64, +) -> Vec { + let is_empty = centroids.is_empty(); + let is_single_value = total_weight == 1; + let mut total_size = if is_empty || is_single_value { + // Preamble, serial version, family, k, flags, and two unused bytes. + size_of::() + } else { + // The short header plus centroid and buffered-value counts. + size_of::() * 2 + }; + if is_single_value { + total_size += size_of::(); + } else if !is_empty { + total_size += size_of::() * 2; + total_size += centroids.len() * (size_of::() + size_of::()); + } + + let mut bytes = SketchBytes::with_capacity(total_size); + bytes.write_u8(if is_empty || is_single_value { + PREAMBLE_LONGS_EMPTY_OR_SINGLE + } else { + PREAMBLE_LONGS_MULTIPLE + }); + bytes.write_u8(SERIAL_VERSION); + bytes.write_u8(Family::TDIGEST.id); + bytes.write_u16_le(k); + bytes.write_u8({ + let mut flags = 0; + if is_empty { + flags |= FLAGS_IS_EMPTY; + } + if is_single_value { + flags |= FLAGS_IS_SINGLE_VALUE; + } + if reverse_merge { + flags |= FLAGS_REVERSE_MERGE; + } + flags + }); + bytes.write_u16_le(0); // unused + if is_empty { + return bytes.into_bytes(); + } + if is_single_value { + bytes.write_f64_le(min); + return bytes.into_bytes(); + } + bytes.write_u32_le(centroids.len() as u32); + bytes.write_u32_le(0); // no buffered values + bytes.write_f64_le(min); + bytes.write_f64_le(max); + for centroid in centroids { + bytes.write_f64_le(centroid.mean); + bytes.write_u64_le(centroid.weight.get()); + } + bytes.into_bytes() +} + /// Immutable (frozen) T-Digest sketch for estimating quantiles and ranks. /// /// See the [module level documentation](super) for more. @@ -1007,6 +1007,46 @@ impl TDigest { self.centroids_weight } + /// Serializes this immutable t-digest to bytes. + /// + /// # Examples + /// + /// ``` + /// use datasketches::tdigest::TDigest; + /// use datasketches::tdigest::TDigestMut; + /// + /// let mut sketch = TDigestMut::new(100).unwrap(); + /// sketch.update(1.0); + /// let digest = sketch.freeze(); + /// let bytes = digest.serialize(); + /// let decoded = TDigest::deserialize(&bytes).unwrap(); + /// assert_eq!(decoded.max_value(), Some(1.0)); + /// ``` + pub fn serialize(&self) -> Vec { + serialize_compressed( + self.k, + self.reverse_merge, + self.min, + self.max, + &self.centroids, + self.centroids_weight, + ) + } + + /// Deserializes an immutable t-digest from the standard double-precision format. + /// + /// The format of the [reference implementation](https://github.com/tdunning/t-digest) is + /// auto-detected. Use [`deserialize_f32()`](Self::deserialize_f32) for the compact + /// DataSketches C++ `tdigest` format. + pub fn deserialize(bytes: &[u8]) -> Result { + Ok(TDigestMut::deserialize(bytes)?.freeze()) + } + + /// Deserializes an immutable t-digest from the compact single-precision DataSketches format. + pub fn deserialize_f32(bytes: &[u8]) -> Result { + Ok(TDigestMut::deserialize_f32(bytes)?.freeze()) + } + fn view(&self) -> TDigestView<'_> { TDigestView { min: self.min, diff --git a/tests-integration/tests/serde_tests/tdigest.rs b/tests-integration/tests/serde_tests/tdigest.rs index 1011df2f..c5bd0fe1 100644 --- a/tests-integration/tests/serde_tests/tdigest.rs +++ b/tests-integration/tests/serde_tests/tdigest.rs @@ -18,6 +18,7 @@ use std::fs; use std::path::PathBuf; +use datasketches::tdigest::TDigest; use datasketches::tdigest::TDigestMut; use googletest::assert_that; use googletest::prelude::all; @@ -226,6 +227,21 @@ fn test_many_values() { assert_eq!(td.quantile(0.5), deserialized_td.quantile(0.5)); } +#[test] +fn test_frozen_roundtrip() { + let tdigest = patterned_digest(100, 1000, 7); + let expected = tdigest.freeze(); + + let bytes = expected.serialize(); + let actual = TDigest::deserialize(&bytes).unwrap(); + + assert_eq!(actual.k(), expected.k()); + assert_eq!(actual.total_weight(), expected.total_weight()); + assert_eq!(actual.min_value(), expected.min_value()); + assert_eq!(actual.max_value(), expected.max_value()); + assert_eq!(actual.quantile(0.5), expected.quantile(0.5)); +} + #[test] fn test_serialized_bytes_stable_for_full_and_merged_digests() { let mut full_buffer = patterned_digest(200, 1_641, 0); From 57fc384fcf7a994e64a1bc8cfc133d190ee1c4aa Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:05:11 +0800 Subject: [PATCH 12/37] refactor(cpc): remove union test hook `CpcUnion::num_coupons` exposed whether the operator currently stored an accumulator sketch or a bit matrix solely so one integration test could compare internal counts. Keeping that hook made the union representation part of the public compatibility surface. Remove the hook and assert behavior through `to_sketch`: the regression proves it reaches Sliding flavor and that its estimate agrees with a sketch over the same stream. Callers needing CPC diagnostics can inspect the resulting `CpcSketch`. --- CHANGELOG.md | 1 + datasketches/src/cpc/union.rs | 13 ------------- tests-integration/tests/cpc_test/union.rs | 1 - 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61e64b9b..3340d4e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All significant changes to this project will be documented in this file. * `FrequentItemsSketch::new` now rejects map sizes below the minimum of 8 instead of silently rounding them up. * Replace `FrequentItemsSketch::epsilon_for_lg` with the fallible `epsilon_for_max_map_size`, and change `apriori_error` to accept the same maximum map size plus an unsigned stream weight. These helpers now match the constructor's units, and `max_map_size` exposes the configured value. * Replace the `is_f32` flag on `TDigestMut::deserialize` with separate `deserialize` and `deserialize_f32` entry points, making the serialized precision explicit at the call site. +* Remove `CpcUnion::num_coupons`, which exposed internal union state solely for tests. Inspect the resulting `CpcSketch` when diagnostics are needed. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/cpc/union.rs b/datasketches/src/cpc/union.rs index 065e1e74..51742a30 100644 --- a/datasketches/src/cpc/union.rs +++ b/datasketches/src/cpc/union.rs @@ -356,19 +356,6 @@ impl CpcUnion { } } -// testing methods -impl CpcUnion { - /// Returns the number of coupons in the union. - /// - /// This is primarily for testing and validation purposes. - pub fn num_coupons(&self) -> u32 { - match &self.state { - UnionState::Accumulator(sketch) => sketch.num_coupons, - UnionState::BitMatrix(matrix) => count_bits_set_in_matrix(matrix), - } - } -} - fn or_window_into_matrix( dst_matrix: &mut [u64], dst_lg_k: u8, diff --git a/tests-integration/tests/cpc_test/union.rs b/tests-integration/tests/cpc_test/union.rs index 3b38e83e..91801652 100644 --- a/tests-integration/tests/cpc_test/union.rs +++ b/tests-integration/tests/cpc_test/union.rs @@ -98,7 +98,6 @@ fn test_sliding_union_matches_single_sketch() { let result = union.to_sketch(); assert!(!result.is_empty()); assert!(result.num_coupons() >= 27 * (1 << 11) / 8); - assert_eq!(result.num_coupons(), union.num_coupons()); let estimate = sketch.estimate(); assert_that!( result.estimate(), From 104d6b8e1ae893c654b2a89b3eb43e222bd1c121 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:05:33 +0800 Subject: [PATCH 13/37] refactor(req): remove hidden diagnostic APIs `#[doc(hidden)]` does not make a `pub` method private or exempt it from semantic-versioning commitments. These four REQ methods exposed compactor levels, nominal capacities, retained counts, and a recomputed weight without a supported caller contract, and no production or integration code used them. Remove the unused diagnostics before release so compactor layout can evolve without preserving test scaffolding as public API. Supported queries, iteration, serialization, and observable sketch state are unchanged. --- CHANGELOG.md | 1 + datasketches/src/req/sketch.rs | 32 -------------------------------- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3340d4e8..46d0d8b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ All significant changes to this project will be documented in this file. * Replace `FrequentItemsSketch::epsilon_for_lg` with the fallible `epsilon_for_max_map_size`, and change `apriori_error` to accept the same maximum map size plus an unsigned stream weight. These helpers now match the constructor's units, and `max_map_size` exposes the configured value. * Replace the `is_f32` flag on `TDigestMut::deserialize` with separate `deserialize` and `deserialize_f32` entry points, making the serialized precision explicit at the call site. * Remove `CpcUnion::num_coupons`, which exposed internal union state solely for tests. Inspect the resulting `CpcSketch` when diagnostics are needed. +* Remove the hidden REQ diagnostic methods `level_info`, `total_nominal_capacity`, `total_retained_items`, and `computed_total_weight`; they exposed implementation details and had no supported caller contract. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/req/sketch.rs b/datasketches/src/req/sketch.rs index e2f1d944..c5e17882 100644 --- a/datasketches/src/req/sketch.rs +++ b/datasketches/src/req/sketch.rs @@ -438,38 +438,6 @@ where } } - /// Returns per-level info: `(level_index, num_items, capacity, weight)`. - /// Internal/test API; subject to change. - #[doc(hidden)] - pub fn level_info(&self) -> Vec<(usize, u32, u32, u64)> { - self.compactors - .iter() - .enumerate() - .map(|(i, c)| (i, c.num_items(), c.nominal_capacity(), c.weight())) - .collect() - } - - /// Total nominal capacity across all levels. Internal/test API. - #[doc(hidden)] - pub fn total_nominal_capacity(&self) -> u32 { - self.compactors.iter().map(|c| c.nominal_capacity()).sum() - } - - /// Total retained items across all levels. Internal/test API. - #[doc(hidden)] - pub fn total_retained_items(&self) -> u32 { - self.compactors.iter().map(|c| c.num_items()).sum() - } - - /// Sum of `level_items × level_weight` across compactors. Internal/test API. - #[doc(hidden)] - pub fn computed_total_weight(&self) -> u64 { - self.compactors - .iter() - .map(|c| c.num_items() as u64 * c.weight()) - .sum() - } - fn flags_byte(&self) -> u8 { let mut flags = 0u8; if self.is_empty() { From 040c46c4b86515fe48b5df069dab67b8e0b54d8d Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:06:30 +0800 Subject: [PATCH 14/37] refactor(theta): colocate Jaccard result types Theta and Tuple Jaccard operators returned a type that callers had to import from the implementation-oriented `thetacommon` module. That leaked the internal sharing arrangement and made each sketch family appear incomplete. Re-export the same `JaccardSimilarity` type from both `theta` and `tuple`, and make the common module crate-private. Only the import path changes; the result representation and operator behavior remain identical. --- CHANGELOG.md | 1 + datasketches/src/lib.rs | 2 +- datasketches/src/thetafamily/mod.rs | 2 +- datasketches/src/thetafamily/theta/mod.rs | 1 + datasketches/src/thetafamily/tuple/mod.rs | 1 + tests-integration/tests/theta_test/jaccard_similarity.rs | 2 +- tests-integration/tests/tuple_test/jaccard_similarity.rs | 2 +- 7 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46d0d8b8..5062ca83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ All significant changes to this project will be documented in this file. * Replace the `is_f32` flag on `TDigestMut::deserialize` with separate `deserialize` and `deserialize_f32` entry points, making the serialized precision explicit at the call site. * Remove `CpcUnion::num_coupons`, which exposed internal union state solely for tests. Inspect the resulting `CpcSketch` when diagnostics are needed. * Remove the hidden REQ diagnostic methods `level_info`, `total_nominal_capacity`, `total_retained_items`, and `computed_total_weight`; they exposed implementation details and had no supported caller contract. +* Move `JaccardSimilarity` from the internal-looking `thetacommon` module to both `theta` and `tuple`, and make `thetacommon` private. Import the result type from the same sketch-family module as its operator. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/lib.rs b/datasketches/src/lib.rs index 066b2376..100ec408 100644 --- a/datasketches/src/lib.rs +++ b/datasketches/src/lib.rs @@ -48,7 +48,7 @@ pub mod tdigest; #[cfg(any(feature = "theta", feature = "tuple"))] mod thetafamily; #[cfg(any(feature = "theta", feature = "tuple"))] -pub use self::thetafamily::common as thetacommon; +pub(crate) use self::thetafamily::common as thetacommon; #[cfg(feature = "theta")] pub use self::thetafamily::theta; #[cfg(feature = "tuple")] diff --git a/datasketches/src/thetafamily/mod.rs b/datasketches/src/thetafamily/mod.rs index 6f4d8cc3..908c4c9b 100644 --- a/datasketches/src/thetafamily/mod.rs +++ b/datasketches/src/thetafamily/mod.rs @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -pub mod common; +pub(crate) mod common; #[cfg(feature = "theta")] pub mod theta; #[cfg(feature = "tuple")] diff --git a/datasketches/src/thetafamily/theta/mod.rs b/datasketches/src/thetafamily/theta/mod.rs index c65fa09e..2cb42e1d 100644 --- a/datasketches/src/thetafamily/theta/mod.rs +++ b/datasketches/src/thetafamily/theta/mod.rs @@ -59,3 +59,4 @@ pub use self::sketch::ThetaSketchBuilder; pub use self::sketch::ThetaSketchView; pub use self::union::ThetaUnion; pub use self::union::ThetaUnionBuilder; +pub use crate::thetafamily::common::JaccardSimilarity; diff --git a/datasketches/src/thetafamily/tuple/mod.rs b/datasketches/src/thetafamily/tuple/mod.rs index c057ee54..db0a9ef7 100644 --- a/datasketches/src/thetafamily/tuple/mod.rs +++ b/datasketches/src/thetafamily/tuple/mod.rs @@ -65,3 +65,4 @@ pub use self::sketch::TupleSketchBuilder; pub use self::sketch::TupleSketchView; pub use self::union::TupleUnion; pub use self::union::TupleUnionBuilder; +pub use crate::thetafamily::common::JaccardSimilarity; diff --git a/tests-integration/tests/theta_test/jaccard_similarity.rs b/tests-integration/tests/theta_test/jaccard_similarity.rs index 2a44b676..da9527be 100644 --- a/tests-integration/tests/theta_test/jaccard_similarity.rs +++ b/tests-integration/tests/theta_test/jaccard_similarity.rs @@ -15,10 +15,10 @@ // specific language governing permissions and limitations // under the License. +use datasketches::theta::JaccardSimilarity; use datasketches::theta::ThetaJaccardSimilarity; use datasketches::theta::ThetaSketch; use datasketches::theta::ThetaSketchBuilder; -use datasketches::thetacommon::JaccardSimilarity; use googletest::assert_that; use googletest::prelude::anything; use googletest::prelude::err; diff --git a/tests-integration/tests/tuple_test/jaccard_similarity.rs b/tests-integration/tests/tuple_test/jaccard_similarity.rs index bbe2e6ca..8b859822 100644 --- a/tests-integration/tests/tuple_test/jaccard_similarity.rs +++ b/tests-integration/tests/tuple_test/jaccard_similarity.rs @@ -15,8 +15,8 @@ // specific language governing permissions and limitations // under the License. -use datasketches::thetacommon::JaccardSimilarity; use datasketches::tuple::DefaultUpdatePolicy; +use datasketches::tuple::JaccardSimilarity; use datasketches::tuple::TupleJaccardSimilarity; use datasketches::tuple::TupleSketchBuilder; use googletest::assert_that; From 907a767bd98898a405dee0362b0cc8699ed9b2e6 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:07:26 +0800 Subject: [PATCH 15/37] refactor(tuple): hide storage entry type `TupleEntry` was externally reachable only through the re-export in the public `tuple` module. Its defining `hash_table` module is private, so the module boundary already keeps its unrestricted `pub` items internal once that re-export is removed. Remove the re-export without restating the enclosing visibility restriction on the entry, its accessors, or the table alias. Update the compact-sketch documentation and changelog to describe the remaining public iterator contract. --- CHANGELOG.md | 1 + datasketches/src/thetafamily/tuple/mod.rs | 1 - datasketches/src/thetafamily/tuple/sketch.rs | 5 ++--- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5062ca83..cd224729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ All significant changes to this project will be documented in this file. * Remove `CpcUnion::num_coupons`, which exposed internal union state solely for tests. Inspect the resulting `CpcSketch` when diagnostics are needed. * Remove the hidden REQ diagnostic methods `level_info`, `total_nominal_capacity`, `total_retained_items`, and `computed_total_weight`; they exposed implementation details and had no supported caller contract. * Move `JaccardSimilarity` from the internal-looking `thetacommon` module to both `theta` and `tuple`, and make `thetacommon` private. Import the result type from the same sketch-family module as its operator. +* Remove the `TupleEntry` re-export. Tuple sketch iterators already expose retained entries as `(hash, &summary)` pairs without leaking the private storage representation. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. diff --git a/datasketches/src/thetafamily/tuple/mod.rs b/datasketches/src/thetafamily/tuple/mod.rs index db0a9ef7..9f832d54 100644 --- a/datasketches/src/thetafamily/tuple/mod.rs +++ b/datasketches/src/thetafamily/tuple/mod.rs @@ -50,7 +50,6 @@ mod sketch; mod union; pub use self::a_not_b::TupleANotB; -pub use self::hash_table::TupleEntry; pub use self::intersection::TupleIntersection; pub use self::jaccard_similarity::TupleJaccardSimilarity; pub use self::policy::DefaultUnionPolicy; diff --git a/datasketches/src/thetafamily/tuple/sketch.rs b/datasketches/src/thetafamily/tuple/sketch.rs index 76494ecb..22990219 100644 --- a/datasketches/src/thetafamily/tuple/sketch.rs +++ b/datasketches/src/thetafamily/tuple/sketch.rs @@ -414,9 +414,8 @@ where /// Compact (immutable) Tuple sketch. /// -/// This is the serialization-friendly form: a compact array of retained [`TupleEntry`] values -/// (hash plus summary) plus theta and a 16-bit seed hash. It can be ordered (sorted ascending by -/// hash) or unordered. +/// This is the serialization-friendly form: a compact array of retained hash-summary pairs plus +/// theta and a 16-bit seed hash. It can be ordered (sorted ascending by hash) or unordered. #[derive(Clone, Debug)] pub struct CompactTupleSketch { entries: Vec>, From 845f2ee156f9d2f66606f297c432ccca46f195db Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:08:04 +0800 Subject: [PATCH 16/37] feat(countmin): reset sketches in place Windowed and batched users need to reuse a Count-Min configuration across independent streams. Reconstructing the sketch for every window reallocates the entire counter table even though its dimensions and seed do not change. Add `reset` to zero the existing table and total weight while preserving hashes, buckets, seed, and allocation. The regression verifies the cleared observations, unchanged configuration, and unchanged estimated allocation size. --- CHANGELOG.md | 1 + datasketches/src/countmin/sketch.rs | 20 +++++++++++++++++++ .../tests/countmin_test/sketch.rs | 17 ++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd224729..f0578396 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ All significant changes to this project will be documented in this file. ### New features +* Add `CountMinSketch::reset` for reusing an allocated counter table across independent streams. * `TDigest` can now be serialized and deserialized directly without converting through `TDigestMut` at the call site. * Add Relative Error Quantiles (REQ) sketches behind the `req` feature, including configurable high- or low-rank accuracy, rank, quantile, PMF, and CDF queries, merging, totally ordered custom item types, the `ReqFloat` adapter for non-NaN floating-point values, and C++/Java-compatible serialization. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 484e6c4f..b19aba37 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -134,6 +134,26 @@ impl CountMinSketch { self.total_weight == T::ZERO } + /// Resets this sketch to its initial empty state. + /// + /// Clears all counters while preserving the table allocation and configuration. + /// + /// # Examples + /// + /// ``` + /// use datasketches::countmin::CountMinSketch; + /// + /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); + /// sketch.update("apple"); + /// sketch.reset(); + /// assert!(sketch.is_empty()); + /// assert_eq!(sketch.estimate("apple"), 0); + /// ``` + pub fn reset(&mut self) { + self.counts.fill(T::ZERO); + self.total_weight = T::ZERO; + } + /// Suggests the number of buckets to achieve the given relative error. /// /// # Errors diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index d22cf30c..64425be0 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -224,6 +224,23 @@ fn test_merge() { assert_that!(left.estimate("b"), ge(4)); } +#[test] +fn test_reset_reuses_configuration() { + let mut sketch = CountMinSketch::::with_seed(3, 64, 123).unwrap(); + sketch.update_with_weight("a", 10); + let size = sketch.estimated_size(); + + sketch.reset(); + + assert!(sketch.is_empty()); + assert_eq!(sketch.total_weight(), 0); + assert_eq!(sketch.estimate("a"), 0); + assert_eq!(sketch.num_hashes(), 3); + assert_eq!(sketch.num_buckets(), 64); + assert_eq!(sketch.seed(), 123); + assert_eq!(sketch.estimated_size(), size); +} + #[test] fn test_serialize_deserialize_empty() { let sketch = CountMinSketch::::with_seed(2, 5, 123).unwrap(); From da834a62409300aa5af53cdcd6b42740d87e8b1a Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:08:31 +0800 Subject: [PATCH 17/37] feat(tdigest): reset mutable digests in place Repeated aggregation windows otherwise have to discard a mutable T-Digest and its centroid capacity. Reusing that storage avoids allocation churn, but every piece of distribution state must return to the constructor invariant. Add `reset` to clear centroids and the unmerged tail while restoring extrema, weight, and merge direction; `k` and vector capacity are retained. The regression checks empty queries and metadata, stable estimated allocation size, and a successful update after reuse. --- CHANGELOG.md | 1 + datasketches/src/tdigest/sketch.rs | 28 +++++++++++++++++++ .../tests/tdigest_test/sketch.rs | 21 ++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0578396..ef0e3f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ All significant changes to this project will be documented in this file. ### New features * Add `CountMinSketch::reset` for reusing an allocated counter table across independent streams. +* Add `TDigestMut::reset` for reusing centroid storage across independent streams. * `TDigest` can now be serialized and deserialized directly without converting through `TDigestMut` at the call site. * Add Relative Error Quantiles (REQ) sketches behind the `req` feature, including configurable high- or low-rank accuracy, rank, quantile, PMF, and CDF queries, merging, totally ordered custom item types, the `ReqFloat` adapter for non-NaN floating-point values, and C++/Java-compatible serialization. diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index 613c2c7d..695f2a79 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -70,6 +70,11 @@ impl TDigestBuffer { self.centroids.is_empty() } + fn clear(&mut self) { + self.centroids.clear(); + self.unmerged_tail_len = 0; + } + fn unmerged_len(&self) -> usize { self.unmerged_tail_len } @@ -320,6 +325,29 @@ impl TDigestMut { self.compressed_weight + self.buffer.unmerged_len() as u64 } + /// Resets this t-digest to its initial empty state. + /// + /// Clears all observations while preserving `k` and the centroid allocation for reuse. + /// + /// # Examples + /// + /// ``` + /// use datasketches::tdigest::TDigestMut; + /// + /// let mut sketch = TDigestMut::new(100).unwrap(); + /// sketch.update(1.0); + /// sketch.reset(); + /// assert!(sketch.is_empty()); + /// assert_eq!(sketch.total_weight(), 0); + /// ``` + pub fn reset(&mut self) { + self.reverse_merge = false; + self.min = f64::INFINITY; + self.max = f64::NEG_INFINITY; + self.buffer.clear(); + self.compressed_weight = 0; + } + /// Merges the given t-digest into this one. /// /// # Examples diff --git a/tests-integration/tests/tdigest_test/sketch.rs b/tests-integration/tests/tdigest_test/sketch.rs index c0cced92..fce6728e 100644 --- a/tests-integration/tests/tdigest_test/sketch.rs +++ b/tests-integration/tests/tdigest_test/sketch.rs @@ -68,6 +68,27 @@ fn test_one_value() { assert_eq!(tdigest.quantile(1.0), Some(1.0)); } +#[test] +fn test_reset_reuses_configuration_and_allocation() { + let mut tdigest = TDigestMut::new(100).unwrap(); + for value in 0..1000 { + tdigest.update(value as f64); + } + let size = tdigest.estimated_size(); + + tdigest.reset(); + + assert!(tdigest.is_empty()); + assert_eq!(tdigest.k(), 100); + assert_eq!(tdigest.total_weight(), 0); + assert_eq!(tdigest.min_value(), None); + assert_eq!(tdigest.max_value(), None); + assert_eq!(tdigest.estimated_size(), size); + + tdigest.update(42.0); + assert_eq!(tdigest.quantile(0.5), Some(42.0)); +} + #[test] fn test_empty_split_points_define_one_bin() { let mut tdigest = TDigestMut::new(100).unwrap(); From f25c92c2f9b3cd56bf569567da631d878d63d214 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:09:58 +0800 Subject: [PATCH 18/37] docs: explain sketch selection and hashing compatibility The crate enables no sketch features by default, yet its crate-level documentation did not show how to enable one or distinguish overlapping algorithm families. The compatibility text also risked implying that a portable serialized format made ordinary Rust `Hash` updates portable across languages. Document feature activation, sketch selection by workload, and the exact `hash::value` wrappers needed for strings, floats, and short integers. Also call out the empty-string behavior used by other DataSketches implementations so cross-language unions do not silently represent different inputs. --- README.md | 4 +++- datasketches/src/lib.rs | 45 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 674d361f..4f560161 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,9 @@ See the [API documentation](https://docs.rs/datasketches) for configuration, acc The minimum supported Rust version is 1.86.0. The crate currently supports little-endian targets only. -Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). When values must hash identically across language implementations, use the compatibility wrappers in `hash::value`. +Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). + +Serialization compatibility does not imply that an ordinary Rust `Hash` implementation produces the same update bytes as another language. When sketches must represent the same inputs across implementations, use `hash::value::raw_bytes` for bytes and strings, `canonical_float` for floating-point values, `sign_extend` for short integers passed to HLL or CPC, and `natural_extend` for short integers passed to Bloom filters. Other DataSketches implementations skip empty strings, so skip them before updating when that behavior matters. See the [changelog](CHANGELOG.md) for release notes and migration guidance. diff --git a/datasketches/src/lib.rs b/datasketches/src/lib.rs index 100ec408..8a3959bd 100644 --- a/datasketches/src/lib.rs +++ b/datasketches/src/lib.rs @@ -17,11 +17,48 @@ //! # Apache® DataSketches™ Core Rust Library Component //! -//! The Sketching Core Library provides a range of stochastic streaming algorithms and closely -//! related Rust technologies that are particularly useful when integrating this technology into -//! systems that must deal with massive data. +//! This crate provides compact, mergeable summaries for answering queries over large data streams. +//! It implements a subset of the algorithms available in the other Apache DataSketches language +//! components. //! -//! This library is divided into modules that constitute distinct groups of functionality. +//! ## Enabling sketches +//! +//! Sketch implementations are opt-in Cargo features; this crate enables none by default. Enable +//! only the algorithms an application uses: +//! +//! ```text +//! cargo add datasketches --features hll,theta +//! ``` +//! +//! Each feature exposes a same-named module. For example, `hll` exposes `datasketches::hll` and +//! `tdigest` exposes `datasketches::tdigest`. +//! +//! ## Choosing a sketch +//! +//! * Use `bloom` for probabilistic membership queries. +//! * Use `countmin` for point-frequency estimates and `frequencies` for discovering heavy hitters. +//! * Use `hll` for fast distinct counts, `cpc` for compact serialized distinct counts, or `theta` +//! when set operations are required. +//! * Use `req` or `tdigest` for ranks and quantiles. REQ targets configurable high- or low-rank +//! accuracy; T-Digest emphasizes distribution tails. +//! * Use `tuple` when retained Theta keys need application-defined summaries. +//! +//! See each module's documentation for accuracy, memory, serialization, and update examples. +//! +//! ## Cross-language hashing +//! +//! Compatible serialization does not by itself make ordinary Rust [`Hash`](std::hash::Hash) +//! input compatible with Java, C++, or Go. Rust strings and slices include type-specific framing, +//! and short integers require different widening rules for different sketch families. When +//! sketches must represent the same updates across languages, use the wrappers in [`hash::value`]: +//! +//! * `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. +//! +//! Other DataSketches implementations skip empty strings rather than hashing them. Check for empty +//! input before updating when that cross-language behavior is required. #![cfg_attr(docsrs, feature(doc_cfg))] #![deny(missing_docs)] From 88e6ea10ae0e1b866a878704e4600a2ccbebf3cf Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:11:07 +0800 Subject: [PATCH 19/37] fix(countmin): saturate counter arithmetic Count-Min supports narrow integer counters, but ordinary addition panicked on overflow in debug builds and wrapped in release builds. Taking the absolute value of the minimum signed weight had the same debug overflow, making results depend on the build profile. Add private saturating absolute-value and addition operations for every supported counter type, and use them for updates, total weight, merges, and upper bounds. Regressions cover unsigned update and merge saturation plus the minimum signed weight. --- CHANGELOG.md | 1 + datasketches/src/countmin/sketch.rs | 12 +++++----- datasketches/src/countmin/value.rs | 23 +++++++++++++------ .../tests/countmin_test/sketch.rs | 23 +++++++++++++++++++ 4 files changed, 46 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef0e3f9e..928c8298 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ All significant changes to this project will be documented in this file. ### Bug fixes +* Count-Min updates, merges, and upper bounds now saturate at the configured counter type's limits instead of panicking in debug builds or wrapping in release builds. The minimum signed weight is handled without overflow. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. * Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index b19aba37..5166b506 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -225,13 +225,13 @@ impl CountMinSketch { if weight == T::ZERO { return; } - let abs_weight = weight.abs(); - self.total_weight = self.total_weight + abs_weight; + let abs_weight = weight.saturating_abs(); + self.total_weight = self.total_weight.saturating_add(abs_weight); let num_buckets = self.num_buckets as usize; for (row, seed) in self.hash_seeds.iter().enumerate() { let bucket = self.bucket_index(&item, *seed); let index = row * num_buckets + bucket; - self.counts[index] = self.counts[index] + weight; + self.counts[index] = self.counts[index].saturating_add(weight); } } @@ -269,7 +269,7 @@ impl CountMinSketch { pub fn upper_bound(&self, item: I) -> T { let estimate = self.estimate(item); let error = self.total_weight.scale(self.relative_error()); - estimate + error + estimate.saturating_add(error) } /// Merges another sketch into this one. @@ -302,9 +302,9 @@ impl CountMinSketch { )); } for (count, other_count) in self.counts.iter_mut().zip(&other.counts) { - *count = *count + *other_count; + *count = count.saturating_add(*other_count); } - self.total_weight = self.total_weight + other.total_weight; + self.total_weight = self.total_weight.saturating_add(other.total_weight); Ok(()) } diff --git a/datasketches/src/countmin/value.rs b/datasketches/src/countmin/value.rs index 70fd5427..9f21b041 100644 --- a/datasketches/src/countmin/value.rs +++ b/datasketches/src/countmin/value.rs @@ -28,16 +28,15 @@ pub trait CountMinValue: private::CountMinValue {} pub trait UnsignedCountMinValue: CountMinValue + private::UnsignedCountMinValue {} mod private { - use std::ops::Add; - use crate::error::Error; - pub trait CountMinValue: Sized + Copy + Ord + Add { + pub trait CountMinValue: Sized + Copy + Ord { const ZERO: Self; const ONE: Self; const MAX: Self; - fn abs(self) -> Self; + fn saturating_abs(self) -> Self; + fn saturating_add(self, other: Self) -> Self; fn scale(self, factor: f64) -> Self; fn to_bytes(self) -> [u8; 8]; fn try_from_bytes(bytes: [u8; 8]) -> Result; @@ -56,8 +55,13 @@ macro_rules! impl_signed { const MAX: Self = $max; #[inline(always)] - fn abs(self) -> Self { - if self >= 0 { self } else { -self } + fn saturating_abs(self) -> Self { + <$name>::saturating_abs(self) + } + + #[inline(always)] + fn saturating_add(self, other: Self) -> Self { + <$name>::saturating_add(self, other) } #[inline(always)] @@ -102,10 +106,15 @@ macro_rules! impl_unsigned { const MAX: Self = $max; #[inline(always)] - fn abs(self) -> Self { + fn saturating_abs(self) -> Self { self } + #[inline(always)] + fn saturating_add(self, other: Self) -> Self { + <$name>::saturating_add(self, other) + } + #[inline(always)] fn scale(self, factor: f64) -> Self { ((self as f64) * factor).trunc() as $name diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index 64425be0..f1a80609 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -159,6 +159,29 @@ fn test_negative_weights() { assert_eq!(sketch.total_weight(), 3); } +#[test] +fn test_arithmetic_saturates_at_value_bounds() { + let mut unsigned = CountMinSketch::::new(2, 8).unwrap(); + unsigned.update_with_weight("x", 250); + unsigned.update_with_weight("x", 10); + assert_eq!(unsigned.estimate("x"), u8::MAX); + assert_eq!(unsigned.total_weight(), u8::MAX); + assert_eq!(unsigned.upper_bound("x"), u8::MAX); + + let mut left = CountMinSketch::::new(2, 8).unwrap(); + left.update_with_weight("x", 250); + let mut right = CountMinSketch::::new(2, 8).unwrap(); + right.update_with_weight("x", 10); + left.merge(&right).unwrap(); + assert_eq!(left.estimate("x"), u8::MAX); + assert_eq!(left.total_weight(), u8::MAX); + + let mut signed = CountMinSketch::::new(2, 8).unwrap(); + signed.update_with_weight("x", i8::MIN); + assert_eq!(signed.estimate("x"), i8::MIN); + assert_eq!(signed.total_weight(), i8::MAX); +} + #[test] fn test_halve() { let buckets = CountMinSketch::::suggest_num_buckets(0.01).unwrap(); From e49a579245585ea8d8b5e064cd54ea464ceaddbc Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:14:19 +0800 Subject: [PATCH 20/37] fix(frequencies): saturate count arithmetic Frequent-items used unchecked `u64` addition in tracked counters, stream weight, purge offsets, merges, and reported upper bounds. Large weighted updates therefore panicked in debug builds but wrapped to small values in release builds, invalidating frequency ordering and error reports. Saturate every count path at `u64::MAX` and remove the impossible positive-count assertion after the zero check. The regression creates a nonzero purge offset, applies a maximum weight through owned and borrowed updates, merges another sketch, and verifies all query forms remain saturated. --- CHANGELOG.md | 1 + .../reverse_purge_item_hash_map.rs | 4 +-- datasketches/src/frequencies/sketch.rs | 25 +++++++++-------- .../tests/frequencies_test/update.rs | 27 +++++++++++++++++++ 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 928c8298..e03ce46a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ All significant changes to this project will be documented in this file. ### Bug fixes +* Frequent-items counters, stream weights, error offsets, and reported upper bounds now saturate at `u64::MAX` instead of panicking in debug builds or wrapping in release builds. * Count-Min updates, merges, and upper bounds now saturate at the configured counter type's limits instead of panicking in debug builds or wrapping in release builds. The minimum signed weight is handled without overflow. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. * Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. diff --git a/datasketches/src/frequencies/reverse_purge_item_hash_map.rs b/datasketches/src/frequencies/reverse_purge_item_hash_map.rs index f32e9ab1..3205f18b 100644 --- a/datasketches/src/frequencies/reverse_purge_item_hash_map.rs +++ b/datasketches/src/frequencies/reverse_purge_item_hash_map.rs @@ -84,7 +84,7 @@ impl ReversePurgeItemHashMap { self.states[probe] = drift as u16; self.num_active += 1; } else { - self.values[probe] += adjust_amount; + self.values[probe] = self.values[probe].saturating_add(adjust_amount); } } @@ -101,7 +101,7 @@ impl ReversePurgeItemHashMap { self.states[probe] = drift as u16; self.num_active += 1; } else { - self.values[probe] += adjust_amount; + self.values[probe] = self.values[probe].saturating_add(adjust_amount); } } diff --git a/datasketches/src/frequencies/sketch.rs b/datasketches/src/frequencies/sketch.rs index d8c765bb..1376e345 100644 --- a/datasketches/src/frequencies/sketch.rs +++ b/datasketches/src/frequencies/sketch.rs @@ -140,6 +140,7 @@ impl Row { /// /// The sketch tracks approximate item frequencies and can return estimates with /// guaranteed upper and lower bounds. +/// Count arithmetic saturates at [`u64::MAX`]. /// /// See the [module level documentation](super) for an overview and error guarantees. #[derive(Debug, Clone)] @@ -196,7 +197,7 @@ impl FrequentItemsSketch { self.hash_map.num_active() } - /// Returns the total weight of the stream. + /// Returns the total weight of the stream, saturated at [`u64::MAX`]. /// /// This is the sum of all counts passed to `update` and `update_with_count`. pub fn total_weight(&self) -> u64 { @@ -222,7 +223,11 @@ impl FrequentItemsSketch { Q: Eq + Hash + ?Sized, { let value = self.hash_map.get(item); - if value > 0 { value + self.offset } else { 0 } + if value > 0 { + value.saturating_add(self.offset) + } else { + 0 + } } /// Returns the guaranteed lower bound frequency for an item. @@ -246,7 +251,7 @@ impl FrequentItemsSketch { T: Borrow, Q: Eq + Hash + ?Sized, { - self.hash_map.get(item) + self.offset + self.hash_map.get(item).saturating_add(self.offset) } /// Returns an upper bound on the maximum error of [`FrequentItemsSketch::estimate`] @@ -365,8 +370,7 @@ impl FrequentItemsSketch { if count == 0 { return; } - assert!(count > 0, "count may not be negative"); - self.stream_weight += count; + self.stream_weight = self.stream_weight.saturating_add(count); self.hash_map.adjust_or_put_value(item, count); self.maybe_resize_or_purge(); } @@ -418,8 +422,7 @@ impl FrequentItemsSketch { if count == 0 { return; } - assert!(count > 0, "count may not be negative"); - self.stream_weight += count; + self.stream_weight = self.stream_weight.saturating_add(count); self.hash_map.adjust_or_put_value_ref(item, count); self.maybe_resize_or_purge(); } @@ -448,11 +451,11 @@ impl FrequentItemsSketch { if other.is_initial_state() { return; } - let merged_total = self.stream_weight + other.stream_weight; + let merged_total = self.stream_weight.saturating_add(other.stream_weight); for (item, count) in other.hash_map.iter() { self.update_with_count_ref(item, count); } - self.offset += other.offset; + self.offset = self.offset.saturating_add(other.offset); self.stream_weight = merged_total; } @@ -515,7 +518,7 @@ impl FrequentItemsSketch { let mut rows = vec![]; for (item, count) in self.hash_map.iter() { let lower = count; - let upper = count + self.offset; + let upper = count.saturating_add(self.offset); let include = match error_type { ErrorType::NoFalseNegatives => upper > threshold, ErrorType::NoFalsePositives => lower > threshold, @@ -540,7 +543,7 @@ impl FrequentItemsSketch { self.cur_map_cap = self.hash_map.capacity(); } else { let delta = self.hash_map.purge(self.sample_size); - self.offset += delta; + self.offset = self.offset.saturating_add(delta); if self.hash_map.num_active() > self.maximum_map_capacity() { panic!("purge did not reduce number of active items"); } diff --git a/tests-integration/tests/frequencies_test/update.rs b/tests-integration/tests/frequencies_test/update.rs index 7eb16fb6..1bef8ef2 100644 --- a/tests-integration/tests/frequencies_test/update.rs +++ b/tests-integration/tests/frequencies_test/update.rs @@ -64,6 +64,33 @@ fn test_items_update_with_zero_count_is_noop() { assert_eq!(sketch.num_active_items(), 0); } +#[test] +fn test_count_arithmetic_saturates() { + let mut sketch = FrequentItemsSketch::new(8).unwrap(); + for item in 0..7 { + sketch.update(item); + } + assert_eq!(sketch.maximum_error(), 1); + + sketch.update_with_count(99, u64::MAX); + sketch.update_ref(&99); + + let mut other = FrequentItemsSketch::new(8).unwrap(); + other.update(99); + sketch.merge(&other); + + assert_eq!(sketch.total_weight(), u64::MAX); + assert_eq!(sketch.lower_bound(&99), u64::MAX); + assert_eq!(sketch.estimate(&99), u64::MAX); + assert_eq!(sketch.upper_bound(&99), u64::MAX); + + let rows = sketch.frequent_items(ErrorType::NoFalseNegatives); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].estimate(), u64::MAX); + assert_eq!(rows[0].upper_bound(), u64::MAX); + assert_eq!(rows[0].lower_bound(), u64::MAX); +} + #[test] fn test_capacity_and_epsilon_helpers() { let longs: FrequentItemsSketch = FrequentItemsSketch::new(8).unwrap(); From 2fce7c20215e774d5145dde03e33c06e780f2bd8 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:16:23 +0800 Subject: [PATCH 21/37] fix(bloom): reject unattainable accuracy targets The accuracy builder promised a target false-positive probability but silently clamped a required bit count to the serialization maximum. For sufficiently large inputs it could therefore build a filter that could not meet the requested probability, while still attempting a very large allocation. Compare the calculated bit requirement with the format limit before converting or allocating, and return `InvalidArgument` when the target is not representable. The regression covers both the public suggestion helper and the builder path. --- CHANGELOG.md | 1 + datasketches/src/bloom/sketch.rs | 16 ++++++++++++---- tests-integration/tests/bloom_test/sketch.rs | 11 +++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e03ce46a..3021e7c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ All significant changes to this project will be documented in this file. ### Bug fixes +* Bloom filter accuracy construction now rejects targets that exceed the maximum serialized filter size instead of silently reducing capacity and violating the requested false-positive probability. * Frequent-items counters, stream weights, error offsets, and reported upper bounds now saturate at `u64::MAX` instead of panicking in debug builds or wrapping in release builds. * Count-Min updates, merges, and upper bounds now saturate at the configured counter type's limits instead of panicking in debug builds or wrapping in release builds. The minimum signed weight is handled without overflow. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. diff --git a/datasketches/src/bloom/sketch.rs b/datasketches/src/bloom/sketch.rs index 1fbf7a3b..30832aa5 100644 --- a/datasketches/src/bloom/sketch.rs +++ b/datasketches/src/bloom/sketch.rs @@ -727,7 +727,8 @@ impl BloomFilterBuilder { /// # Errors /// /// Returns an error if the configured accuracy or size parameters are outside their supported - /// ranges. + /// ranges, or if the requested accuracy requires a filter larger than the serialized format + /// supports. pub fn build(self) -> Result { let (num_bits, num_hashes) = match self.mode { BloomFilterBuilderMode::Accuracy { max_items, fpp } => { @@ -776,7 +777,8 @@ impl BloomFilterBuilder { /// /// # Errors /// - /// Returns an error if `max_items` is zero or `fpp` is outside `(0.0, 1.0]`. + /// Returns an error if `max_items` is zero, `fpp` is outside `(0.0, 1.0]`, or the target + /// accuracy requires a filter larger than the serialized format supports. /// /// # Examples /// @@ -798,9 +800,15 @@ impl BloomFilterBuilder { let p = fpp; let ln2_squared = std::f64::consts::LN_2 * std::f64::consts::LN_2; - let bits = (-n * p.ln() / ln2_squared).ceil() as u64; + let bits = (-n * p.ln() / ln2_squared).ceil(); + if bits > Self::MAX_NUM_BITS as f64 { + return Err(Error::invalid_argument(format!( + "target accuracy requires {bits:.0} bits, but at most {} are supported", + Self::MAX_NUM_BITS + ))); + } - Ok(bits.clamp(Self::MIN_NUM_BITS, Self::MAX_NUM_BITS)) + Ok((bits as u64).max(Self::MIN_NUM_BITS)) } /// Suggests optimal number of hash functions given max items and bit count. diff --git a/tests-integration/tests/bloom_test/sketch.rs b/tests-integration/tests/bloom_test/sketch.rs index 92bcfd4a..8a1008d5 100644 --- a/tests-integration/tests/bloom_test/sketch.rs +++ b/tests-integration/tests/bloom_test/sketch.rs @@ -192,3 +192,14 @@ fn test_parameter_suggestions_validate_inputs() { .all(|error| error.kind() == ErrorKind::InvalidArgument) ); } + +#[test] +fn test_accuracy_builder_rejects_unrepresentable_target() { + let error = BloomFilterBuilder::suggest_num_bits(u64::MAX, 0.01).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + + let error = BloomFilterBuilder::with_accuracy(u64::MAX, 0.01) + .build() + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); +} From a0a9cb2f9c94c4257a6b536e5bac454c4afcffb6 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:17:16 +0800 Subject: [PATCH 22/37] docs(cpc): clarify diagnostic methods `CpcSketch::validate` and `num_coupons` match supported CPC diagnostics, but the source grouped them as testing methods and did not explain their cost or interpretation. A caller could mistake the coupon count for the cardinality estimate or run validation on a hot path. Document that validation reconstructs a bit matrix proportional to `k`, and that coupons are an internal statistic rather than the distinct-count estimate. This changes documentation only. --- datasketches/src/cpc/sketch.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/datasketches/src/cpc/sketch.rs b/datasketches/src/cpc/sketch.rs index 0a6ce00f..df70db06 100644 --- a/datasketches/src/cpc/sketch.rs +++ b/datasketches/src/cpc/sketch.rs @@ -927,20 +927,21 @@ impl CpcSketch { } } -// testing methods impl CpcSketch { - /// Returns `true` if the sketch's internal state is valid. + /// Checks whether the stored coupon count agrees with the sketch's reconstructed bit matrix. /// - /// This is primarily for testing and validation purposes. + /// This integrity check allocates a bit matrix proportional to the configured `k`, so it is + /// intended for diagnostics rather than a hot query path. pub fn validate(&self) -> bool { let bit_matrix = self.build_bit_matrix(); let num_bits_set = count_bits_set_in_matrix(&bit_matrix); num_bits_set == self.num_coupons } - /// Returns the number of coupons in the sketch. + /// Returns the number of distinct CPC coupons collected by the sketch. /// - /// This is primarily for testing and validation purposes. + /// The coupon count is an internal statistic, not a cardinality estimate. Use + /// [`estimate()`](Self::estimate) for the estimated number of distinct input values. pub fn num_coupons(&self) -> u32 { self.num_coupons } From b696cabd93aeb579ad82d3aa8ce1495a05070a99 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 21:20:53 +0800 Subject: [PATCH 23/37] chore(deps): update chacha20 to a non-yanked release `cargo package` warned that the lockfile selected yanked `chacha20` 0.10.1 through `rand` 0.10.2. Shipping a release branch with that warning makes dependency resolution look stale even though the manifest constraint remains valid. Update only the lockfile entry to MSRV-compatible `chacha20` 0.10.2. Manifest requirements and the public dependency graph are unchanged, and package verification completes without the yank warning. --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cb1b0c9e..87ec3e30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -155,9 +155,9 @@ checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" [[package]] name = "chacha20" -version = "0.10.1" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" dependencies = [ "cfg-if", "cpufeatures", From bbca047ebba0d5b953bd0b021218071ce8e88d49 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:25:12 +0800 Subject: [PATCH 24/37] fixup Signed-off-by: tison --- datasketches/src/frequencies/sketch.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/datasketches/src/frequencies/sketch.rs b/datasketches/src/frequencies/sketch.rs index 1376e345..e61e803d 100644 --- a/datasketches/src/frequencies/sketch.rs +++ b/datasketches/src/frequencies/sketch.rs @@ -39,12 +39,12 @@ type SerializeItem = fn(&mut SketchBytes, &T); type DeserializeItems = fn(SketchSlice<'_>, usize) -> Result, Error>; const LG_MIN_MAP_SIZE: u8 = 3; -const MIN_MAP_SIZE: usize = 1usize << LG_MIN_MAP_SIZE; +const MIN_MAP_SIZE: usize = 1 << LG_MIN_MAP_SIZE; // Java represents map sizes as positive `int` powers of two, while the C++ // implementation uses 32-bit table indices. Keep Rust configurations within // the same cross-language range. const LG_MAX_MAP_SIZE: u8 = 30; -const MAX_MAP_SIZE: usize = 1usize << LG_MAX_MAP_SIZE; +const MAX_MAP_SIZE: usize = 1 << LG_MAX_MAP_SIZE; const SAMPLE_SIZE: usize = 1024; const EPSILON_FACTOR: f64 = 3.5; const LOAD_FACTOR_NUMERATOR: usize = 3; @@ -52,7 +52,7 @@ const LOAD_FACTOR_DENOMINATOR: usize = 4; fn map_capacity_for_lg(lg_map_size: u8) -> usize { debug_assert!(lg_map_size <= LG_MAX_MAP_SIZE); - (1usize << lg_map_size) * LOAD_FACTOR_NUMERATOR / LOAD_FACTOR_DENOMINATOR + (1 << lg_map_size) * LOAD_FACTOR_NUMERATOR / LOAD_FACTOR_DENOMINATOR } fn lg_for_max_map_size(max_map_size: usize) -> Result { @@ -320,7 +320,7 @@ impl FrequentItemsSketch { /// Returns the configured maximum map size. pub fn max_map_size(&self) -> usize { - 1usize << self.lg_max_map_size + 1 << self.lg_max_map_size } /// Returns the configured `lg_max_map_size`. @@ -562,7 +562,7 @@ impl FrequentItemsSketch { lg_cur <= lg_max, "lg_cur_map_size must not exceed lg_max_map_size" ); - let map = ReversePurgeItemHashMap::new(1usize << lg_cur); + let map = ReversePurgeItemHashMap::new(1 << lg_cur); let cur_map_cap = map.capacity(); let max_map_cap = map_capacity_for_lg(lg_max); let sample_size = SAMPLE_SIZE.min(max_map_cap); From cac164d8610e6c0aa7ad3cfeb5b9f76e8d6d8f3f Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:27:50 +0800 Subject: [PATCH 25/37] revert(theta): preserve shared Jaccard API `thetacommon::JaccardSimilarity` is intentionally the shared result type for both Theta and Tuple operators. Re-exporting it from each family module created duplicate public paths and incorrectly privatized a supported namespace. Restore the public `thetacommon` entry point and the original test imports, and remove the migration note from the changelog. Jaccard computation and result representation are unchanged. --- CHANGELOG.md | 1 - datasketches/src/lib.rs | 2 +- datasketches/src/thetafamily/mod.rs | 2 +- datasketches/src/thetafamily/theta/mod.rs | 1 - datasketches/src/thetafamily/tuple/mod.rs | 1 - tests-integration/tests/theta_test/jaccard_similarity.rs | 2 +- tests-integration/tests/tuple_test/jaccard_similarity.rs | 2 +- 7 files changed, 4 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3021e7c2..e1ba99a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,6 @@ All significant changes to this project will be documented in this file. * Replace the `is_f32` flag on `TDigestMut::deserialize` with separate `deserialize` and `deserialize_f32` entry points, making the serialized precision explicit at the call site. * Remove `CpcUnion::num_coupons`, which exposed internal union state solely for tests. Inspect the resulting `CpcSketch` when diagnostics are needed. * Remove the hidden REQ diagnostic methods `level_info`, `total_nominal_capacity`, `total_retained_items`, and `computed_total_weight`; they exposed implementation details and had no supported caller contract. -* Move `JaccardSimilarity` from the internal-looking `thetacommon` module to both `theta` and `tuple`, and make `thetacommon` private. Import the result type from the same sketch-family module as its operator. * Remove the `TupleEntry` re-export. Tuple sketch iterators already expose retained entries as `(hash, &summary)` pairs without leaking the private storage representation. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. diff --git a/datasketches/src/lib.rs b/datasketches/src/lib.rs index 8a3959bd..1f36e79a 100644 --- a/datasketches/src/lib.rs +++ b/datasketches/src/lib.rs @@ -85,7 +85,7 @@ pub mod tdigest; #[cfg(any(feature = "theta", feature = "tuple"))] mod thetafamily; #[cfg(any(feature = "theta", feature = "tuple"))] -pub(crate) use self::thetafamily::common as thetacommon; +pub use self::thetafamily::common as thetacommon; #[cfg(feature = "theta")] pub use self::thetafamily::theta; #[cfg(feature = "tuple")] diff --git a/datasketches/src/thetafamily/mod.rs b/datasketches/src/thetafamily/mod.rs index 908c4c9b..6f4d8cc3 100644 --- a/datasketches/src/thetafamily/mod.rs +++ b/datasketches/src/thetafamily/mod.rs @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -pub(crate) mod common; +pub mod common; #[cfg(feature = "theta")] pub mod theta; #[cfg(feature = "tuple")] diff --git a/datasketches/src/thetafamily/theta/mod.rs b/datasketches/src/thetafamily/theta/mod.rs index 2cb42e1d..c65fa09e 100644 --- a/datasketches/src/thetafamily/theta/mod.rs +++ b/datasketches/src/thetafamily/theta/mod.rs @@ -59,4 +59,3 @@ pub use self::sketch::ThetaSketchBuilder; pub use self::sketch::ThetaSketchView; pub use self::union::ThetaUnion; pub use self::union::ThetaUnionBuilder; -pub use crate::thetafamily::common::JaccardSimilarity; diff --git a/datasketches/src/thetafamily/tuple/mod.rs b/datasketches/src/thetafamily/tuple/mod.rs index 9f832d54..9c3fcf12 100644 --- a/datasketches/src/thetafamily/tuple/mod.rs +++ b/datasketches/src/thetafamily/tuple/mod.rs @@ -64,4 +64,3 @@ pub use self::sketch::TupleSketchBuilder; pub use self::sketch::TupleSketchView; pub use self::union::TupleUnion; pub use self::union::TupleUnionBuilder; -pub use crate::thetafamily::common::JaccardSimilarity; diff --git a/tests-integration/tests/theta_test/jaccard_similarity.rs b/tests-integration/tests/theta_test/jaccard_similarity.rs index da9527be..2a44b676 100644 --- a/tests-integration/tests/theta_test/jaccard_similarity.rs +++ b/tests-integration/tests/theta_test/jaccard_similarity.rs @@ -15,10 +15,10 @@ // specific language governing permissions and limitations // under the License. -use datasketches::theta::JaccardSimilarity; use datasketches::theta::ThetaJaccardSimilarity; use datasketches::theta::ThetaSketch; use datasketches::theta::ThetaSketchBuilder; +use datasketches::thetacommon::JaccardSimilarity; use googletest::assert_that; use googletest::prelude::anything; use googletest::prelude::err; diff --git a/tests-integration/tests/tuple_test/jaccard_similarity.rs b/tests-integration/tests/tuple_test/jaccard_similarity.rs index 8b859822..bbe2e6ca 100644 --- a/tests-integration/tests/tuple_test/jaccard_similarity.rs +++ b/tests-integration/tests/tuple_test/jaccard_similarity.rs @@ -15,8 +15,8 @@ // specific language governing permissions and limitations // under the License. +use datasketches::thetacommon::JaccardSimilarity; use datasketches::tuple::DefaultUpdatePolicy; -use datasketches::tuple::JaccardSimilarity; use datasketches::tuple::TupleJaccardSimilarity; use datasketches::tuple::TupleSketchBuilder; use googletest::assert_that; From a767cb18cef2a920b47c667103ce1d0a8a6af2a2 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:28:57 +0800 Subject: [PATCH 26/37] style: omit inferred usize suffixes These literals are constrained to `usize` by their comparison, collection, return, branch, or function-argument context. Removing the suffixes keeps the expressions shorter without changing their inferred types. Keep explicit suffixes where Rust needs them for method resolution or where the integer width affects shift semantics. --- datasketches/src/cpc/sketch.rs | 2 +- datasketches/src/frequencies/reverse_purge_item_hash_map.rs | 2 +- datasketches/src/req/sketch.rs | 2 +- datasketches/src/thetafamily/tuple/sketch.rs | 2 +- tests-integration/tests/frequencies_test/update.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/datasketches/src/cpc/sketch.rs b/datasketches/src/cpc/sketch.rs index df70db06..3f083e89 100644 --- a/datasketches/src/cpc/sketch.rs +++ b/datasketches/src/cpc/sketch.rs @@ -754,7 +754,7 @@ impl CpcSketch { table_num_entries, table_data_words ))); } - let k = 1usize << lg_k; + let k = 1 << lg_k; if has_window && window_data_words.saturating_mul(32) < k { return Err(Error::deserial(format!( "window data ({} words) is too short for lg_k = {lg_k}", diff --git a/datasketches/src/frequencies/reverse_purge_item_hash_map.rs b/datasketches/src/frequencies/reverse_purge_item_hash_map.rs index 3205f18b..b3ced706 100644 --- a/datasketches/src/frequencies/reverse_purge_item_hash_map.rs +++ b/datasketches/src/frequencies/reverse_purge_item_hash_map.rs @@ -141,7 +141,7 @@ impl ReversePurgeItemHashMap { pub fn purge(&mut self, sample_size: usize) -> u64 { let limit = sample_size.min(self.num_active).min(MAX_SAMPLE_SIZE); let mut samples = Vec::with_capacity(limit); - let mut i = 0usize; + let mut i = 0; while samples.len() < limit { if self.is_active(i) { samples.push(self.values[i]); diff --git a/datasketches/src/req/sketch.rs b/datasketches/src/req/sketch.rs index c5e17882..af93c6c4 100644 --- a/datasketches/src/req/sketch.rs +++ b/datasketches/src/req/sketch.rs @@ -466,7 +466,7 @@ where { // Fixed sketch preamble: 8 bytes (preamble_ints, serial_version, family, // flags, k(2), num_levels, num_raw_items). - let mut size = 8usize; + let mut size = 8; if self.is_empty() { return size; } diff --git a/datasketches/src/thetafamily/tuple/sketch.rs b/datasketches/src/thetafamily/tuple/sketch.rs index 22990219..e13a462b 100644 --- a/datasketches/src/thetafamily/tuple/sketch.rs +++ b/datasketches/src/thetafamily/tuple/sketch.rs @@ -676,7 +676,7 @@ impl CompactTupleSketch { let mut theta = MAX_THETA; let num_entries = if pre_longs == 1 { - 1usize + 1 } else { let n = cursor .read_u32_le() diff --git a/tests-integration/tests/frequencies_test/update.rs b/tests-integration/tests/frequencies_test/update.rs index 1bef8ef2..18840fdb 100644 --- a/tests-integration/tests/frequencies_test/update.rs +++ b/tests-integration/tests/frequencies_test/update.rs @@ -594,7 +594,7 @@ fn test_invalid_map_size_returns_error() { #[test] fn test_map_size_above_cross_language_limit_returns_error() { - let error = FrequentItemsSketch::::new(1usize << 31).unwrap_err(); + let error = FrequentItemsSketch::::new(1 << 31).unwrap_err(); assert_eq!(error.kind(), ErrorKind::InvalidArgument); } From 85a79770edcc250a71521376c9de4960043579a4 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:38:40 +0800 Subject: [PATCH 27/37] revert(countmin): remove speculative reset API `CountMinSketch::reset` was added for a hypothetical allocation-reuse workload rather than an existing caller, issue, compatibility requirement, or reference implementation. Remove the public method, its dedicated test, and its unreleased changelog entry. A reset contract can be designed when a concrete workload demonstrates that reconstruction is insufficient. --- CHANGELOG.md | 1 - datasketches/src/countmin/sketch.rs | 20 ------------------- .../tests/countmin_test/sketch.rs | 17 ---------------- 3 files changed, 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1ba99a1..50bdb266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,6 @@ All significant changes to this project will be documented in this file. ### New features -* Add `CountMinSketch::reset` for reusing an allocated counter table across independent streams. * Add `TDigestMut::reset` for reusing centroid storage across independent streams. * `TDigest` can now be serialized and deserialized directly without converting through `TDigestMut` at the call site. * Add Relative Error Quantiles (REQ) sketches behind the `req` feature, including configurable high- or low-rank accuracy, rank, quantile, PMF, and CDF queries, merging, totally ordered custom item types, the `ReqFloat` adapter for non-NaN floating-point values, and C++/Java-compatible serialization. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 5166b506..5e14e052 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -134,26 +134,6 @@ impl CountMinSketch { self.total_weight == T::ZERO } - /// Resets this sketch to its initial empty state. - /// - /// Clears all counters while preserving the table allocation and configuration. - /// - /// # Examples - /// - /// ``` - /// use datasketches::countmin::CountMinSketch; - /// - /// let mut sketch = CountMinSketch::::new(4, 128).unwrap(); - /// sketch.update("apple"); - /// sketch.reset(); - /// assert!(sketch.is_empty()); - /// assert_eq!(sketch.estimate("apple"), 0); - /// ``` - pub fn reset(&mut self) { - self.counts.fill(T::ZERO); - self.total_weight = T::ZERO; - } - /// Suggests the number of buckets to achieve the given relative error. /// /// # Errors diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index f1a80609..a03157ff 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -247,23 +247,6 @@ fn test_merge() { assert_that!(left.estimate("b"), ge(4)); } -#[test] -fn test_reset_reuses_configuration() { - let mut sketch = CountMinSketch::::with_seed(3, 64, 123).unwrap(); - sketch.update_with_weight("a", 10); - let size = sketch.estimated_size(); - - sketch.reset(); - - assert!(sketch.is_empty()); - assert_eq!(sketch.total_weight(), 0); - assert_eq!(sketch.estimate("a"), 0); - assert_eq!(sketch.num_hashes(), 3); - assert_eq!(sketch.num_buckets(), 64); - assert_eq!(sketch.seed(), 123); - assert_eq!(sketch.estimated_size(), size); -} - #[test] fn test_serialize_deserialize_empty() { let sketch = CountMinSketch::::with_seed(2, 5, 123).unwrap(); From ab74492aa7c0ea6aec77c36d665a44d13be6c67c Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:38:59 +0800 Subject: [PATCH 28/37] revert(tdigest): remove speculative reset API `TDigestMut::reset` was introduced for an assumed window-reuse optimization without a concrete caller, issue, compatibility requirement, or corresponding API in the DataSketches Java and C++ implementations. Remove the public method, its reset-only buffer helper, the dedicated test, and its unreleased changelog entry. Revisit the API only when a real workload can define the required reuse and allocation guarantees. --- CHANGELOG.md | 1 - datasketches/src/tdigest/sketch.rs | 28 ------------------- .../tests/tdigest_test/sketch.rs | 21 -------------- 3 files changed, 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50bdb266..ea77b2f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,6 @@ All significant changes to this project will be documented in this file. ### New features -* Add `TDigestMut::reset` for reusing centroid storage across independent streams. * `TDigest` can now be serialized and deserialized directly without converting through `TDigestMut` at the call site. * Add Relative Error Quantiles (REQ) sketches behind the `req` feature, including configurable high- or low-rank accuracy, rank, quantile, PMF, and CDF queries, merging, totally ordered custom item types, the `ReqFloat` adapter for non-NaN floating-point values, and C++/Java-compatible serialization. diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index 695f2a79..613c2c7d 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -70,11 +70,6 @@ impl TDigestBuffer { self.centroids.is_empty() } - fn clear(&mut self) { - self.centroids.clear(); - self.unmerged_tail_len = 0; - } - fn unmerged_len(&self) -> usize { self.unmerged_tail_len } @@ -325,29 +320,6 @@ impl TDigestMut { self.compressed_weight + self.buffer.unmerged_len() as u64 } - /// Resets this t-digest to its initial empty state. - /// - /// Clears all observations while preserving `k` and the centroid allocation for reuse. - /// - /// # Examples - /// - /// ``` - /// use datasketches::tdigest::TDigestMut; - /// - /// let mut sketch = TDigestMut::new(100).unwrap(); - /// sketch.update(1.0); - /// sketch.reset(); - /// assert!(sketch.is_empty()); - /// assert_eq!(sketch.total_weight(), 0); - /// ``` - pub fn reset(&mut self) { - self.reverse_merge = false; - self.min = f64::INFINITY; - self.max = f64::NEG_INFINITY; - self.buffer.clear(); - self.compressed_weight = 0; - } - /// Merges the given t-digest into this one. /// /// # Examples diff --git a/tests-integration/tests/tdigest_test/sketch.rs b/tests-integration/tests/tdigest_test/sketch.rs index fce6728e..c0cced92 100644 --- a/tests-integration/tests/tdigest_test/sketch.rs +++ b/tests-integration/tests/tdigest_test/sketch.rs @@ -68,27 +68,6 @@ fn test_one_value() { assert_eq!(tdigest.quantile(1.0), Some(1.0)); } -#[test] -fn test_reset_reuses_configuration_and_allocation() { - let mut tdigest = TDigestMut::new(100).unwrap(); - for value in 0..1000 { - tdigest.update(value as f64); - } - let size = tdigest.estimated_size(); - - tdigest.reset(); - - assert!(tdigest.is_empty()); - assert_eq!(tdigest.k(), 100); - assert_eq!(tdigest.total_weight(), 0); - assert_eq!(tdigest.min_value(), None); - assert_eq!(tdigest.max_value(), None); - assert_eq!(tdigest.estimated_size(), size); - - tdigest.update(42.0); - assert_eq!(tdigest.quantile(0.5), Some(42.0)); -} - #[test] fn test_empty_split_points_define_one_bin() { let mut tdigest = TDigestMut::new(100).unwrap(); From 6d042a79d178bb561ef126e2ccc0c3e7ad466304 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:49:29 +0800 Subject: [PATCH 29/37] docs(cpc): hide test-only inspection methods The coupon counter and full-state validator are exposed so integration tests can inspect CPC invariants. They are not part of the supported query API: Java keeps the coupon count package-private, while C++ marks both hooks as private/debugging helpers. Keep the methods callable for the existing external test crate, but exclude them from generated API documentation. --- datasketches/src/cpc/sketch.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/datasketches/src/cpc/sketch.rs b/datasketches/src/cpc/sketch.rs index 3f083e89..8599e46d 100644 --- a/datasketches/src/cpc/sketch.rs +++ b/datasketches/src/cpc/sketch.rs @@ -928,20 +928,20 @@ impl CpcSketch { } impl CpcSketch { - /// Checks whether the stored coupon count agrees with the sketch's reconstructed bit matrix. + /// Returns `true` if the sketch's internal state is valid. /// - /// This integrity check allocates a bit matrix proportional to the configured `k`, so it is - /// intended for diagnostics rather than a hot query path. + /// This is intended for testing and validation purposes. + #[doc(hidden)] pub fn validate(&self) -> bool { let bit_matrix = self.build_bit_matrix(); let num_bits_set = count_bits_set_in_matrix(&bit_matrix); num_bits_set == self.num_coupons } - /// Returns the number of distinct CPC coupons collected by the sketch. + /// Returns the number of coupons in the sketch. /// - /// The coupon count is an internal statistic, not a cardinality estimate. Use - /// [`estimate()`](Self::estimate) for the estimated number of distinct input values. + /// This is intended for testing and validation purposes. + #[doc(hidden)] pub fn num_coupons(&self) -> u32 { self.num_coupons } From 170d34c7f7f37020ace60842444099f3776b6a13 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:50:41 +0800 Subject: [PATCH 30/37] revert(countmin): remove saturating arithmetic Saturating signed counters is not a valid overflow policy for a mergeable sketch. It is non-associative when positive and negative weights mix, so update order and merge grouping can change estimates. The saturating absolute value of the minimum signed integer also silently undercounts a magnitude that the value type cannot represent. Restore the previous arithmetic until overflow behavior is designed as an explicit API contract. This reverts commit 88e6ea1. --- CHANGELOG.md | 1 - datasketches/src/countmin/sketch.rs | 12 +++++----- datasketches/src/countmin/value.rs | 23 ++++++------------- .../tests/countmin_test/sketch.rs | 23 ------------------- 4 files changed, 13 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea77b2f9..e645f9a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,6 @@ All significant changes to this project will be documented in this file. * Bloom filter accuracy construction now rejects targets that exceed the maximum serialized filter size instead of silently reducing capacity and violating the requested false-positive probability. * Frequent-items counters, stream weights, error offsets, and reported upper bounds now saturate at `u64::MAX` instead of panicking in debug builds or wrapping in release builds. -* Count-Min updates, merges, and upper bounds now saturate at the configured counter type's limits instead of panicking in debug builds or wrapping in release builds. The minimum signed weight is handled without overflow. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. * Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. diff --git a/datasketches/src/countmin/sketch.rs b/datasketches/src/countmin/sketch.rs index 5e14e052..484e6c4f 100644 --- a/datasketches/src/countmin/sketch.rs +++ b/datasketches/src/countmin/sketch.rs @@ -205,13 +205,13 @@ impl CountMinSketch { if weight == T::ZERO { return; } - let abs_weight = weight.saturating_abs(); - self.total_weight = self.total_weight.saturating_add(abs_weight); + let abs_weight = weight.abs(); + self.total_weight = self.total_weight + abs_weight; let num_buckets = self.num_buckets as usize; for (row, seed) in self.hash_seeds.iter().enumerate() { let bucket = self.bucket_index(&item, *seed); let index = row * num_buckets + bucket; - self.counts[index] = self.counts[index].saturating_add(weight); + self.counts[index] = self.counts[index] + weight; } } @@ -249,7 +249,7 @@ impl CountMinSketch { pub fn upper_bound(&self, item: I) -> T { let estimate = self.estimate(item); let error = self.total_weight.scale(self.relative_error()); - estimate.saturating_add(error) + estimate + error } /// Merges another sketch into this one. @@ -282,9 +282,9 @@ impl CountMinSketch { )); } for (count, other_count) in self.counts.iter_mut().zip(&other.counts) { - *count = count.saturating_add(*other_count); + *count = *count + *other_count; } - self.total_weight = self.total_weight.saturating_add(other.total_weight); + self.total_weight = self.total_weight + other.total_weight; Ok(()) } diff --git a/datasketches/src/countmin/value.rs b/datasketches/src/countmin/value.rs index 9f21b041..70fd5427 100644 --- a/datasketches/src/countmin/value.rs +++ b/datasketches/src/countmin/value.rs @@ -28,15 +28,16 @@ pub trait CountMinValue: private::CountMinValue {} pub trait UnsignedCountMinValue: CountMinValue + private::UnsignedCountMinValue {} mod private { + use std::ops::Add; + use crate::error::Error; - pub trait CountMinValue: Sized + Copy + Ord { + pub trait CountMinValue: Sized + Copy + Ord + Add { const ZERO: Self; const ONE: Self; const MAX: Self; - fn saturating_abs(self) -> Self; - fn saturating_add(self, other: Self) -> Self; + fn abs(self) -> Self; fn scale(self, factor: f64) -> Self; fn to_bytes(self) -> [u8; 8]; fn try_from_bytes(bytes: [u8; 8]) -> Result; @@ -55,13 +56,8 @@ macro_rules! impl_signed { const MAX: Self = $max; #[inline(always)] - fn saturating_abs(self) -> Self { - <$name>::saturating_abs(self) - } - - #[inline(always)] - fn saturating_add(self, other: Self) -> Self { - <$name>::saturating_add(self, other) + fn abs(self) -> Self { + if self >= 0 { self } else { -self } } #[inline(always)] @@ -106,15 +102,10 @@ macro_rules! impl_unsigned { const MAX: Self = $max; #[inline(always)] - fn saturating_abs(self) -> Self { + fn abs(self) -> Self { self } - #[inline(always)] - fn saturating_add(self, other: Self) -> Self { - <$name>::saturating_add(self, other) - } - #[inline(always)] fn scale(self, factor: f64) -> Self { ((self as f64) * factor).trunc() as $name diff --git a/tests-integration/tests/countmin_test/sketch.rs b/tests-integration/tests/countmin_test/sketch.rs index a03157ff..d22cf30c 100644 --- a/tests-integration/tests/countmin_test/sketch.rs +++ b/tests-integration/tests/countmin_test/sketch.rs @@ -159,29 +159,6 @@ fn test_negative_weights() { assert_eq!(sketch.total_weight(), 3); } -#[test] -fn test_arithmetic_saturates_at_value_bounds() { - let mut unsigned = CountMinSketch::::new(2, 8).unwrap(); - unsigned.update_with_weight("x", 250); - unsigned.update_with_weight("x", 10); - assert_eq!(unsigned.estimate("x"), u8::MAX); - assert_eq!(unsigned.total_weight(), u8::MAX); - assert_eq!(unsigned.upper_bound("x"), u8::MAX); - - let mut left = CountMinSketch::::new(2, 8).unwrap(); - left.update_with_weight("x", 250); - let mut right = CountMinSketch::::new(2, 8).unwrap(); - right.update_with_weight("x", 10); - left.merge(&right).unwrap(); - assert_eq!(left.estimate("x"), u8::MAX); - assert_eq!(left.total_weight(), u8::MAX); - - let mut signed = CountMinSketch::::new(2, 8).unwrap(); - signed.update_with_weight("x", i8::MIN); - assert_eq!(signed.estimate("x"), i8::MIN); - assert_eq!(signed.total_weight(), i8::MAX); -} - #[test] fn test_halve() { let buckets = CountMinSketch::::suggest_num_buckets(0.01).unwrap(); From de01332e570c4b35509dfc6a018de70c1a3cc3b0 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 22:51:00 +0800 Subject: [PATCH 31/37] revert(frequencies): remove saturating arithmetic Saturating u64 counters silently changes the sketch from tracking mathematical stream frequencies to tracking capped frequencies. Once the true count exceeds u64::MAX, the documented upper-bound guarantee can no longer hold even though the returned value looks valid. Restore the previous arithmetic until overflow behavior is defined explicitly by the public API. This reverts commit e49a579. --- CHANGELOG.md | 1 - .../reverse_purge_item_hash_map.rs | 4 +-- datasketches/src/frequencies/sketch.rs | 25 ++++++++--------- .../tests/frequencies_test/update.rs | 27 ------------------- 4 files changed, 13 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e645f9a1..7782ee52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,6 @@ All significant changes to this project will be documented in this file. ### Bug fixes * Bloom filter accuracy construction now rejects targets that exceed the maximum serialized filter size instead of silently reducing capacity and violating the requested false-positive probability. -* Frequent-items counters, stream weights, error offsets, and reported upper bounds now saturate at `u64::MAX` instead of panicking in debug builds or wrapping in release builds. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. * Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. diff --git a/datasketches/src/frequencies/reverse_purge_item_hash_map.rs b/datasketches/src/frequencies/reverse_purge_item_hash_map.rs index b3ced706..ce3d62dc 100644 --- a/datasketches/src/frequencies/reverse_purge_item_hash_map.rs +++ b/datasketches/src/frequencies/reverse_purge_item_hash_map.rs @@ -84,7 +84,7 @@ impl ReversePurgeItemHashMap { self.states[probe] = drift as u16; self.num_active += 1; } else { - self.values[probe] = self.values[probe].saturating_add(adjust_amount); + self.values[probe] += adjust_amount; } } @@ -101,7 +101,7 @@ impl ReversePurgeItemHashMap { self.states[probe] = drift as u16; self.num_active += 1; } else { - self.values[probe] = self.values[probe].saturating_add(adjust_amount); + self.values[probe] += adjust_amount; } } diff --git a/datasketches/src/frequencies/sketch.rs b/datasketches/src/frequencies/sketch.rs index e61e803d..7a520453 100644 --- a/datasketches/src/frequencies/sketch.rs +++ b/datasketches/src/frequencies/sketch.rs @@ -140,7 +140,6 @@ impl Row { /// /// The sketch tracks approximate item frequencies and can return estimates with /// guaranteed upper and lower bounds. -/// Count arithmetic saturates at [`u64::MAX`]. /// /// See the [module level documentation](super) for an overview and error guarantees. #[derive(Debug, Clone)] @@ -197,7 +196,7 @@ impl FrequentItemsSketch { self.hash_map.num_active() } - /// Returns the total weight of the stream, saturated at [`u64::MAX`]. + /// Returns the total weight of the stream. /// /// This is the sum of all counts passed to `update` and `update_with_count`. pub fn total_weight(&self) -> u64 { @@ -223,11 +222,7 @@ impl FrequentItemsSketch { Q: Eq + Hash + ?Sized, { let value = self.hash_map.get(item); - if value > 0 { - value.saturating_add(self.offset) - } else { - 0 - } + if value > 0 { value + self.offset } else { 0 } } /// Returns the guaranteed lower bound frequency for an item. @@ -251,7 +246,7 @@ impl FrequentItemsSketch { T: Borrow, Q: Eq + Hash + ?Sized, { - self.hash_map.get(item).saturating_add(self.offset) + self.hash_map.get(item) + self.offset } /// Returns an upper bound on the maximum error of [`FrequentItemsSketch::estimate`] @@ -370,7 +365,8 @@ impl FrequentItemsSketch { if count == 0 { return; } - self.stream_weight = self.stream_weight.saturating_add(count); + assert!(count > 0, "count may not be negative"); + self.stream_weight += count; self.hash_map.adjust_or_put_value(item, count); self.maybe_resize_or_purge(); } @@ -422,7 +418,8 @@ impl FrequentItemsSketch { if count == 0 { return; } - self.stream_weight = self.stream_weight.saturating_add(count); + assert!(count > 0, "count may not be negative"); + self.stream_weight += count; self.hash_map.adjust_or_put_value_ref(item, count); self.maybe_resize_or_purge(); } @@ -451,11 +448,11 @@ impl FrequentItemsSketch { if other.is_initial_state() { return; } - let merged_total = self.stream_weight.saturating_add(other.stream_weight); + let merged_total = self.stream_weight + other.stream_weight; for (item, count) in other.hash_map.iter() { self.update_with_count_ref(item, count); } - self.offset = self.offset.saturating_add(other.offset); + self.offset += other.offset; self.stream_weight = merged_total; } @@ -518,7 +515,7 @@ impl FrequentItemsSketch { let mut rows = vec![]; for (item, count) in self.hash_map.iter() { let lower = count; - let upper = count.saturating_add(self.offset); + let upper = count + self.offset; let include = match error_type { ErrorType::NoFalseNegatives => upper > threshold, ErrorType::NoFalsePositives => lower > threshold, @@ -543,7 +540,7 @@ impl FrequentItemsSketch { self.cur_map_cap = self.hash_map.capacity(); } else { let delta = self.hash_map.purge(self.sample_size); - self.offset = self.offset.saturating_add(delta); + self.offset += delta; if self.hash_map.num_active() > self.maximum_map_capacity() { panic!("purge did not reduce number of active items"); } diff --git a/tests-integration/tests/frequencies_test/update.rs b/tests-integration/tests/frequencies_test/update.rs index 18840fdb..9c1ed001 100644 --- a/tests-integration/tests/frequencies_test/update.rs +++ b/tests-integration/tests/frequencies_test/update.rs @@ -64,33 +64,6 @@ fn test_items_update_with_zero_count_is_noop() { assert_eq!(sketch.num_active_items(), 0); } -#[test] -fn test_count_arithmetic_saturates() { - let mut sketch = FrequentItemsSketch::new(8).unwrap(); - for item in 0..7 { - sketch.update(item); - } - assert_eq!(sketch.maximum_error(), 1); - - sketch.update_with_count(99, u64::MAX); - sketch.update_ref(&99); - - let mut other = FrequentItemsSketch::new(8).unwrap(); - other.update(99); - sketch.merge(&other); - - assert_eq!(sketch.total_weight(), u64::MAX); - assert_eq!(sketch.lower_bound(&99), u64::MAX); - assert_eq!(sketch.estimate(&99), u64::MAX); - assert_eq!(sketch.upper_bound(&99), u64::MAX); - - let rows = sketch.frequent_items(ErrorType::NoFalseNegatives); - assert_eq!(rows.len(), 1); - assert_eq!(rows[0].estimate(), u64::MAX); - assert_eq!(rows[0].upper_bound(), u64::MAX); - assert_eq!(rows[0].lower_bound(), u64::MAX); -} - #[test] fn test_capacity_and_epsilon_helpers() { let longs: FrequentItemsSketch = FrequentItemsSketch::new(8).unwrap(); From 7d029bdc9f4811da30058006068dcfe37a9131f3 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 23:16:20 +0800 Subject: [PATCH 32/37] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4f560161..3143336e 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ The minimum supported Rust version is 1.86.0. The crate currently supports littl Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). -Serialization compatibility does not imply that an ordinary Rust `Hash` implementation produces the same update bytes as another language. When sketches must represent the same inputs across implementations, use `hash::value::raw_bytes` for bytes and strings, `canonical_float` for floating-point values, `sign_extend` for short integers passed to HLL or CPC, and `natural_extend` for short integers passed to Bloom filters. Other DataSketches implementations skip empty strings, so skip them before updating when that behavior matters. +Serialization compatibility does not imply that an ordinary Rust `Hash` implementation produces the same update bytes as another language. When sketches must represent the same inputs across implementations, use `hash::value::{raw_bytes, canonical_float, sign_extend, natural_extend}` (and the constructors within those modules) to match the other language implementations’ hashing rules. Other DataSketches implementations skip empty strings, so skip them before updating when that behavior matters. See the [changelog](CHANGELOG.md) for release notes and migration guidance. From bf1af5fc4972e16bc8c41e8f0770127746b72173 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 23:17:01 +0800 Subject: [PATCH 33/37] refactor(bloom): keep sizing behind the builder The public formula helpers duplicated the accuracy builder and exposed intermediate values that callers could combine inconsistently.\n\nInline target sizing into build, document validation and rounding at the two construction entry points, and retain only behavior-level boundary coverage. --- CHANGELOG.md | 2 +- datasketches/src/bloom/mod.rs | 13 +- datasketches/src/bloom/sketch.rs | 164 +++++-------------- tests-integration/tests/bloom_test/sketch.rs | 36 ++-- 4 files changed, 69 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7782ee52..40a56ad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ All significant changes to this project will be documented in this file. * `BloomFilter::union` and `BloomFilter::intersect` now return `Result`. Callers must handle incompatible filter configurations instead of relying on a panic. * `CountMinSketch::merge` now returns `Result`. Callers must handle incompatible sketch configurations instead of relying on a panic. * `CpcUnion::update` now returns `Result`. Callers must handle seed mismatches instead of relying on a panic. -* `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp` now return `Result` and reject invalid sizing inputs instead of silently clamping them. +* Remove `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp`. Use `with_accuracy(...).build()` for target-based sizing or `with_size(...).build()` for an explicit precomputed configuration. * `CpcSketch::max_serialized_bytes` now returns `Result` and reports an invalid `lg_k` instead of panicking. * `FrequentItemsSketch::new` now rejects map sizes below the minimum of 8 instead of silently rounding them up. * Replace `FrequentItemsSketch::epsilon_for_lg` with the fallible `epsilon_for_max_map_size`, and change `apriori_error` to accept the same maximum map size plus an unsigned stream weight. These helpers now match the constructor's units, and `max_map_size` exposes the configured value. diff --git a/datasketches/src/bloom/mod.rs b/datasketches/src/bloom/mod.rs index be16817e..dabecdbd 100644 --- a/datasketches/src/bloom/mod.rs +++ b/datasketches/src/bloom/mod.rs @@ -63,7 +63,8 @@ //! //! ## By Accuracy (Recommended) //! -//! Automatically calculates optimal size and hash functions: +//! Derive the size and hash-function count from an expected distinct-item count and a target +//! false-positive probability: //! //! ``` //! use datasketches::bloom::BloomFilterBuilder; @@ -77,6 +78,12 @@ //! .unwrap(); //! ``` //! +//! `max_items` is a sizing assumption, not an insertion limit. The filter continues accepting +//! distinct items beyond that count, but its false-positive probability can then exceed the target. +//! Accuracy inputs are validated by `build`: `max_items` must be positive, `fpp` must be in +//! `(0.0, 1.0]`, and the requested target must fit the serialized Bloom filter format. An `fpp` of +//! `1.0` is accepted and creates the smallest allocation: 64 bits and one hash function. +//! //! ## By Size (Manual) //! //! Specify requested bit count and hash functions (rounded up to a multiple of 64 bits): @@ -92,6 +99,10 @@ //! .unwrap(); //! ``` //! +//! Manual construction requires a positive bit count supported by the serialized format and a +//! hash-function count in `1..=32767`. The requested bit count is rounded up to a multiple of 64, +//! which is the value returned by [`BloomFilter::capacity`]. +//! //! # Set Operations //! //! Bloom filters support efficient set operations: diff --git a/datasketches/src/bloom/sketch.rs b/datasketches/src/bloom/sketch.rs index 30832aa5..6a119e91 100644 --- a/datasketches/src/bloom/sketch.rs +++ b/datasketches/src/bloom/sketch.rs @@ -618,6 +618,9 @@ impl BloomFilter { /// * [`with_size()`](Self::with_size): Specify requested bit count and hash functions (manual) /// /// Configuration is stored without validation and checked when [`build()`](Self::build) is called. +/// Accuracy construction treats `max_items` as a sizing assumption, not an insertion limit. The +/// filter continues accepting items beyond that count, but its false-positive probability can then +/// exceed the requested target. #[derive(Debug, Clone)] pub struct BloomFilterBuilder { mode: BloomFilterBuilderMode, @@ -643,11 +646,15 @@ impl BloomFilterBuilder { /// Maximum allowed number of hash functions. const MAX_NUM_HASHES: u16 = i16::MAX as u16; - /// Creates a builder with optimal parameters for a target accuracy. + /// Creates a builder that derives its parameters from a target accuracy. /// - /// Automatically calculates the optimal number of bits and hash functions - /// to achieve the desired false positive probability for a given number of items. - /// The parameters are validated when [`build()`](Self::build) is called. + /// Uses the standard Bloom filter sizing formulas to choose the requested number of bits and + /// hash functions. The parameters are validated when [`build()`](Self::build) is called. + /// + /// `max_items` is the expected maximum number of distinct items, not a hard insertion limit. + /// Inserting more distinct items remains valid but can increase the false-positive probability + /// beyond `fpp`. An `fpp` of `1.0` is accepted and creates the smallest allocation: 64 bits and + /// one hash function. /// /// # Arguments /// @@ -681,6 +688,9 @@ impl BloomFilterBuilder { /// The underlying storage is word-based, so the actual capacity is rounded /// up to the next multiple of 64 bits. /// + /// `num_bits` must be positive and fit the serialized Bloom filter format. `num_hashes` must be + /// in the range `1..=32767`. These constraints are checked by [`build()`](Self::build). + /// /// # Arguments /// /// * `num_bits`: Total number of bits in the filter. @@ -726,14 +736,38 @@ impl BloomFilterBuilder { /// /// # Errors /// - /// Returns an error if the configured accuracy or size parameters are outside their supported - /// ranges, or if the requested accuracy requires a filter larger than the serialized format - /// supports. + /// In accuracy mode, returns an error if `max_items` is zero, `fpp` is outside `(0.0, 1.0]`, or + /// the target requires more bits than the serialized format supports. + /// + /// In manual size mode, returns an error if `num_bits` is zero or exceeds the serialized format + /// limit, or if `num_hashes` is outside `1..=32767`. + /// + /// Valid configurations may still request more memory than the current process can allocate. pub fn build(self) -> Result { let (num_bits, num_hashes) = match self.mode { BloomFilterBuilderMode::Accuracy { max_items, fpp } => { - let num_bits = Self::suggest_num_bits(max_items, fpp)?; - let num_hashes = Self::suggest_num_hashes_from_accuracy(max_items, num_bits)?; + if max_items == 0 { + return Err(Error::invalid_argument("max_items must be greater than 0")); + } + if !(fpp > 0.0 && fpp <= 1.0) { + return Err(Error::invalid_argument("fpp must be in (0.0, 1.0]")); + } + + let n = max_items as f64; + let ln2_squared = std::f64::consts::LN_2 * std::f64::consts::LN_2; + let bits = (-n * fpp.ln() / ln2_squared).ceil(); + if bits > Self::MAX_NUM_BITS as f64 { + return Err(Error::invalid_argument(format!( + "target accuracy requires {bits:.0} bits, but at most {} are supported", + Self::MAX_NUM_BITS + ))); + } + + let num_bits = (bits as u64).max(Self::MIN_NUM_BITS); + let num_hashes = (num_bits as f64 / n * std::f64::consts::LN_2).ceil().clamp( + f64::from(Self::MIN_NUM_HASHES), + f64::from(Self::MAX_NUM_HASHES), + ) as u16; (num_bits, num_hashes) } BloomFilterBuilderMode::Size { @@ -769,116 +803,4 @@ impl BloomFilterBuilder { bit_array, }) } - - /// Suggests optimal number of bits given max items and target FPP. - /// - /// Formula: `m = -n * ln(p) / (ln(2)^2)` - /// where n = max_items, p = fpp - /// - /// # Errors - /// - /// Returns an error if `max_items` is zero, `fpp` is outside `(0.0, 1.0]`, or the target - /// accuracy requires a filter larger than the serialized format supports. - /// - /// # Examples - /// - /// ``` - /// use datasketches::bloom::BloomFilterBuilder; - /// - /// let bits = BloomFilterBuilder::suggest_num_bits(1000, 0.01).unwrap(); - /// assert!(bits > 9000 && bits < 10000); // ~9585 bits - /// ``` - pub fn suggest_num_bits(max_items: u64, fpp: f64) -> Result { - if max_items == 0 { - return Err(Error::invalid_argument("max_items must be greater than 0")); - } - if !(fpp > 0.0 && fpp <= 1.0) { - return Err(Error::invalid_argument("fpp must be in (0.0, 1.0]")); - } - - let n = max_items as f64; - let p = fpp; - let ln2_squared = std::f64::consts::LN_2 * std::f64::consts::LN_2; - - let bits = (-n * p.ln() / ln2_squared).ceil(); - if bits > Self::MAX_NUM_BITS as f64 { - return Err(Error::invalid_argument(format!( - "target accuracy requires {bits:.0} bits, but at most {} are supported", - Self::MAX_NUM_BITS - ))); - } - - Ok((bits as u64).max(Self::MIN_NUM_BITS)) - } - - /// Suggests optimal number of hash functions given max items and bit count. - /// - /// Formula: `k = (m/n) * ln(2)` - /// where m = num_bits, n = max_items - /// - /// # Errors - /// - /// Returns an error if `max_items` is zero or `num_bits` is outside the supported range. - /// - /// # Examples - /// - /// ``` - /// use datasketches::bloom::BloomFilterBuilder; - /// - /// let hashes = BloomFilterBuilder::suggest_num_hashes_from_accuracy(1000, 10000).unwrap(); - /// assert_eq!(hashes, 7); // Optimal k ≈ 6.93 - /// ``` - pub fn suggest_num_hashes_from_accuracy(max_items: u64, num_bits: u64) -> Result { - if max_items == 0 { - return Err(Error::invalid_argument("max_items must be greater than 0")); - } - if !(Self::MIN_NUM_BITS..=Self::MAX_NUM_BITS).contains(&num_bits) { - return Err(Error::invalid_argument(format!( - "num_bits must be between {} and {}, got {}", - Self::MIN_NUM_BITS, - Self::MAX_NUM_BITS, - num_bits - ))); - } - - let m = num_bits as f64; - let n = max_items as f64; - - // Ceil to avoid selecting too few hashes. - let k = (m / n * std::f64::consts::LN_2).ceil(); - Ok(k.clamp( - f64::from(Self::MIN_NUM_HASHES), - f64::from(Self::MAX_NUM_HASHES), - ) as u16) - } - - /// Suggests optimal number of hash functions from target FPP. - /// - /// Formula: `k = -log2(p)` - /// where p = fpp - /// - /// # Errors - /// - /// Returns an error if `fpp` is outside `(0.0, 1.0]`. - /// - /// # Examples - /// - /// ``` - /// use datasketches::bloom::BloomFilterBuilder; - /// - /// let hashes = BloomFilterBuilder::suggest_num_hashes_from_fpp(0.01).unwrap(); - /// assert_eq!(hashes, 7); // -log2(0.01) ≈ 6.64 - /// ``` - pub fn suggest_num_hashes_from_fpp(fpp: f64) -> Result { - if !(fpp > 0.0 && fpp <= 1.0) { - return Err(Error::invalid_argument("fpp must be in (0.0, 1.0]")); - } - - // Ceil to avoid selecting too few hashes. - let k = -fpp.log2(); - Ok(k.ceil().clamp( - f64::from(Self::MIN_NUM_HASHES), - f64::from(Self::MAX_NUM_HASHES), - ) as u16) - } } diff --git a/tests-integration/tests/bloom_test/sketch.rs b/tests-integration/tests/bloom_test/sketch.rs index 8a1008d5..3032fd99 100644 --- a/tests-integration/tests/bloom_test/sketch.rs +++ b/tests-integration/tests/bloom_test/sketch.rs @@ -159,10 +159,19 @@ fn test_accuracy_builder_rejects_zero_items_at_build() { #[test] fn test_accuracy_builder_rejects_invalid_probability_at_build() { - let error = BloomFilterBuilder::with_accuracy(100, 1.5) - .build() - .unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); + for fpp in [0.0, 1.5, f64::NAN] { + let error = BloomFilterBuilder::with_accuracy(100, fpp) + .build() + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } +} + +#[test] +fn test_accuracy_builder_accepts_one_probability() { + let filter = BloomFilterBuilder::with_accuracy(100, 1.0).build().unwrap(); + assert_eq!(filter.capacity(), 64); + assert_eq!(filter.num_hashes(), 1); } #[test] @@ -177,27 +186,8 @@ fn test_size_builder_rejects_zero_hashes_at_build() { assert_eq!(error.kind(), ErrorKind::InvalidArgument); } -#[test] -fn test_parameter_suggestions_validate_inputs() { - let errors = [ - BloomFilterBuilder::suggest_num_bits(0, 0.01).unwrap_err(), - BloomFilterBuilder::suggest_num_bits(1000, f64::NAN).unwrap_err(), - BloomFilterBuilder::suggest_num_hashes_from_accuracy(0, 10_000).unwrap_err(), - BloomFilterBuilder::suggest_num_hashes_from_accuracy(1000, 0).unwrap_err(), - BloomFilterBuilder::suggest_num_hashes_from_fpp(0.0).unwrap_err(), - ]; - assert!( - errors - .iter() - .all(|error| error.kind() == ErrorKind::InvalidArgument) - ); -} - #[test] fn test_accuracy_builder_rejects_unrepresentable_target() { - let error = BloomFilterBuilder::suggest_num_bits(u64::MAX, 0.01).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); - let error = BloomFilterBuilder::with_accuracy(u64::MAX, 0.01) .build() .unwrap_err(); From 592ae318a09241bb9e0c50a546312155943f503f Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 23:23:06 +0800 Subject: [PATCH 34/37] docs(changelog): report net release changes REQ is new since the latest release, so its intermediate diagnostic methods and try_new migration are not release-facing changes.\n\nRemove those development-only details and consolidate the Count-Min suggestion signature and behavior into one migration entry. --- CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40a56ad4..b3302430 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,12 +15,12 @@ All significant changes to this project will be documented in this file. * Replace `FrequentItemsSketch::epsilon_for_lg` with the fallible `epsilon_for_max_map_size`, and change `apriori_error` to accept the same maximum map size plus an unsigned stream weight. These helpers now match the constructor's units, and `max_map_size` exposes the configured value. * Replace the `is_f32` flag on `TDigestMut::deserialize` with separate `deserialize` and `deserialize_f32` entry points, making the serialized precision explicit at the call site. * Remove `CpcUnion::num_coupons`, which exposed internal union state solely for tests. Inspect the resulting `CpcSketch` when diagnostics are needed. -* Remove the hidden REQ diagnostic methods `level_info`, `total_nominal_capacity`, `total_retained_items`, and `computed_total_weight`; they exposed implementation details and had no supported caller contract. * Remove the `TupleEntry` re-export. Tuple sketch iterators already expose retained entries as `(hash, &summary)` pairs without leaking the private storage representation. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. -* Fallible sketch and operator constructors now return `Result` directly from `new` or `with_seed`. `ReqSketch` and `TDigestMut` no longer provide `try_new`, and the Count-Min parameter suggestion methods also return `Result`. +* Parameter-validating constructors on `CountMinSketch`, `CpcSketch`, `CpcUnion`, `FrequentItemsSketch`, `HllSketch`, `HllUnion`, `TDigestMut`, and seeded Theta and Tuple set operators now return `Result` instead of panicking. Use `TDigestMut::new` in place of `TDigestMut::try_new`. +* `CountMinSketch::{suggest_num_buckets, suggest_num_hashes}` now return `Result`, reject invalid or unsupported targets, and return values accepted by the constructors. ### New features @@ -36,7 +36,6 @@ All significant changes to this project will be documented in this file. * Bloom filter accuracy construction now rejects targets that exceed the maximum serialized filter size instead of silently reducing capacity and violating the requested false-positive probability. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. -* Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. * `FrequentItemsSketch` now enforces the cross-language map-size limit of `2^30` consistently. Oversized construction returns `InvalidArgument`, and malformed or oversized serialized images return `InvalidData` instead of panicking or attempting excessive allocation. * T-Digest compression now supports `k = u16::MAX` without overflowing. From c2f01d72dfb53f46ce8182f7ea474f548f67d400 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 23:43:31 +0800 Subject: [PATCH 35/37] docs(changelog): preserve existing Count-Min entries The Count-Min implementation and release notes were already updated together in c25f551. Restore their existing category split so the release-baseline cleanup remains focused on removing development-only REQ history. --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3302430..165f3f30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,7 @@ All significant changes to this project will be documented in this file. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. -* Parameter-validating constructors on `CountMinSketch`, `CpcSketch`, `CpcUnion`, `FrequentItemsSketch`, `HllSketch`, `HllUnion`, `TDigestMut`, and seeded Theta and Tuple set operators now return `Result` instead of panicking. Use `TDigestMut::new` in place of `TDigestMut::try_new`. -* `CountMinSketch::{suggest_num_buckets, suggest_num_hashes}` now return `Result`, reject invalid or unsupported targets, and return values accepted by the constructors. +* Fallible sketch and operator constructors now return `Result` directly from `new` or `with_seed`. `TDigestMut` no longer provides `try_new`, and the Count-Min parameter suggestion methods also return `Result`. ### New features @@ -36,6 +35,7 @@ All significant changes to this project will be documented in this file. * Bloom filter accuracy construction now rejects targets that exceed the maximum serialized filter size instead of silently reducing capacity and violating the requested false-positive probability. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. +* Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. * `FrequentItemsSketch` now enforces the cross-language map-size limit of `2^30` consistently. Oversized construction returns `InvalidArgument`, and malformed or oversized serialized images return `InvalidData` instead of panicking or attempting excessive allocation. * T-Digest compression now supports `k = u16::MAX` without overflowing. From 43d63a6fa8559dd8ac841ad0d1d4d6909706ce3e Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 23:43:57 +0800 Subject: [PATCH 36/37] docs: single-source changelog guidance Keep the detailed policy in CONTRIBUTING.md and make AGENTS.md point to that section. This avoids duplicated wording drifting between the contributor and agent instructions. --- AGENTS.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f5f631db..f8ac5190 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,4 @@ Before planning or modifying this repository, read [CONTRIBUTING.md](CONTRIBUTIN For test changes, pay particular attention to the "Integration test layout" and "Serialization snapshots" sections. Keep the documented workflow synchronized with structural changes, and run the applicable `cargo x check`, `cargo x test`, and `cargo x lint` commands before handing work back. -Apply the changelog guidance in [CONTRIBUTING.md](CONTRIBUTING.md) to every change. Update the permanent `Unreleased` section in the same pull request for significant user-visible behavior, and do not add entries mechanically for excluded maintenance work. - -Treat `CHANGELOG.md` as release notes for users rather than a summary of implementation work. Name the affected API or workload and the observable outcome, and keep performance claims within the scenario supported by evidence. +For every change, follow the [changelog guidance](CONTRIBUTING.md#changelog) in `CONTRIBUTING.md` as the single source of truth. From 13139f2aa6289a5eff8c651673ac9f95c24d8600 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 30 Aug 2026 23:47:10 +0800 Subject: [PATCH 37/37] docs(changelog): classify Count-Min API break The suggestion methods changed their public return types from integers to Result, so callers must update even though the new validation also fixes invalid outputs. Record the complete final behavior once under breaking changes. --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 165f3f30..7e1c7258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All significant changes to this project will be documented in this file. * `BloomFilter::union` and `BloomFilter::intersect` now return `Result`. Callers must handle incompatible filter configurations instead of relying on a panic. * `CountMinSketch::merge` now returns `Result`. Callers must handle incompatible sketch configurations instead of relying on a panic. +* `CountMinSketch::{suggest_num_buckets, suggest_num_hashes}` now return `Result`. Callers must handle invalid or unsupported targets; successful suggestions are valid inputs to `CountMinSketch::new`. * `CpcUnion::update` now returns `Result`. Callers must handle seed mismatches instead of relying on a panic. * Remove `BloomFilterBuilder::suggest_num_bits`, `suggest_num_hashes_from_accuracy`, and `suggest_num_hashes_from_fpp`. Use `with_accuracy(...).build()` for target-based sizing or `with_size(...).build()` for an explicit precomputed configuration. * `CpcSketch::max_serialized_bytes` now returns `Result` and reports an invalid `lg_k` instead of panicking. @@ -19,7 +20,7 @@ All significant changes to this project will be documented in this file. * `ThetaIntersection::to_sketch` and `TupleIntersection::to_sketch` now return `Option`. Callers must handle `None` until the intersection receives its first successful update. * `BloomFilterBuilder`, `ThetaSketchBuilder`, `ThetaUnionBuilder`, `TupleSketchBuilder`, and `TupleUnionBuilder` now validate their configuration when `build` is called, and `build` returns `Result`. Callers must propagate or handle construction errors. * `BloomFilterBuilder::{MIN_NUM_BITS, MAX_NUM_BITS, MIN_NUM_HASHES, MAX_NUM_HASHES}` are no longer public. Callers should pass configurations to `build` and handle `InvalidArgument` instead of prevalidating against these constants. -* Fallible sketch and operator constructors now return `Result` directly from `new` or `with_seed`. `TDigestMut` no longer provides `try_new`, and the Count-Min parameter suggestion methods also return `Result`. +* Fallible sketch and operator constructors now return `Result` directly from `new` or `with_seed`. `TDigestMut` no longer provides `try_new`. ### New features @@ -35,7 +36,6 @@ All significant changes to this project will be documented in this file. * Bloom filter accuracy construction now rejects targets that exceed the maximum serialized filter size instead of silently reducing capacity and violating the requested false-positive probability. * T-Digest CDF and PMF queries now accept an empty split-point slice and return the single all-values bin instead of panicking. -* Count-Min parameter suggestions now return constructor-valid values and reject relative-error targets that require more buckets than the sketch supports. * Bloom filter deserialization now rejects malformed images with inconsistent counts or payload lengths, while valid images with a dirty cached count are restored correctly. * `FrequentItemsSketch` now enforces the cross-language map-size limit of `2^30` consistently. Oversized construction returns `InvalidArgument`, and malformed or oversized serialized images return `InvalidData` instead of panicking or attempting excessive allocation. * T-Digest compression now supports `k = u16::MAX` without overflowing.