From aa5968e9a08d32c8709aa6b8cc1e08c9249d9e6f Mon Sep 17 00:00:00 2001 From: GordonYuanyc Date: Mon, 20 Jul 2026 00:20:11 -0400 Subject: [PATCH] fix(cms): stop clamping RegularPath estimates at i32::MAX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CountMin::estimate on the regular path seeded the running minimum with S::Counter::from(i32::MAX). For counters wider than i32 (i64/i128), any key whose probed cells all exceed 2147483647 never triggered the `v < min` update, so estimate silently returned the seed — capping every large count at exactly 2147483647. Seed the minimum from row 0's probed cell instead, matching the fast path's fast_query_min. Add a regression test covering an i64 sketch with counts of ~35 billion. Co-Authored-By: Claude Opus 4.8 --- src/sketches/countminsketch.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/sketches/countminsketch.rs b/src/sketches/countminsketch.rs index 9fa49d9..4d128ab 100644 --- a/src/sketches/countminsketch.rs +++ b/src/sketches/countminsketch.rs @@ -311,8 +311,11 @@ where pub fn estimate(&self, value: &DataInput) -> S::Counter { let rows = self.counts.rows(); let cols = self.counts.cols(); - let mut min = S::Counter::from(i32::MAX); - for r in 0..rows { + // Seed the running minimum from row 0's probed cell, then fold in the + // rest. Mirrors the fast path's `fast_query_min`. + let col0 = ((H::hash64_seeded(0, value) & LOWER_32_MASK) as usize) % cols; + let mut min = self.counts.query_one_counter(0, col0); + for r in 1..rows { let hashed = H::hash64_seeded(r, value); let col = ((hashed & LOWER_32_MASK) as usize) % cols; let v = self.counts.query_one_counter(r, col); @@ -712,6 +715,25 @@ mod tests { } } + #[test] + fn estimate_does_not_clamp_i64_counts_above_i32_max() { + // Regression: the running minimum used to be seeded with `i32::MAX`, + // which clamped i64 estimates whenever every probed cell exceeded + // 2147483647, capping large counts at 2147483647. + let mut sk = CountMin::, RegularPath>::with_dimensions(3, 64); + let key = DataInput::Str("pkt_len"); + let count: i64 = 35_000_000_000; // ~35 billion, well past i32::MAX + + sk.insert_many(&key, count); + + let est = sk.estimate(&key); + assert!( + est >= count, + "estimate {est} should be at least the true count {count}, not clamped" + ); + assert_ne!(est, i32::MAX as i64, "estimate must not clamp to i32::MAX"); + } + #[test] fn merge_adds_counters_element_wise() { let mut left = CountMin::, RegularPath>::with_dimensions(2, 32);