From 0f632e4afe815ddfcc7d652add1365248c1fdae6 Mon Sep 17 00:00:00 2001 From: tison Date: Mon, 31 Aug 2026 01:18:11 +0800 Subject: [PATCH] refactor(tuple): return named entries from iterators --- CHANGELOG.md | 2 +- .../src/thetafamily/tuple/hash_table.rs | 5 ++-- .../src/thetafamily/tuple/intersection.rs | 2 +- datasketches/src/thetafamily/tuple/mod.rs | 1 + datasketches/src/thetafamily/tuple/sketch.rs | 29 +++++++++---------- tests-integration/tests/serde_tests/tuple.rs | 2 +- tests-integration/tests/tuple_test/a_not_b.rs | 4 +-- .../tests/tuple_test/intersection.rs | 2 +- tests-integration/tests/tuple_test/sketch.rs | 16 ++++++---- tests-integration/tests/tuple_test/union.rs | 4 +-- 10 files changed, 35 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaf8efd7..04d6c0c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,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 `TupleEntry` re-export. Tuple sketch iterators already expose retained entries as `(hash, &summary)` pairs without leaking the private storage representation. +* Tuple sketch iterators now yield `&TupleEntry<_>` values instead of `(hash, &summary)` pairs. Use `entry.hash()` and `entry.summary()` to inspect each retained entry. * `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/hash_table.rs b/datasketches/src/thetafamily/tuple/hash_table.rs index 638d0c7f..1e849591 100644 --- a/datasketches/src/thetafamily/tuple/hash_table.rs +++ b/datasketches/src/thetafamily/tuple/hash_table.rs @@ -101,10 +101,9 @@ impl TupleHashTable { }) } - /// Returns an iterator over retained entries as `(hash, &summary)` pairs. - pub fn iter(&self) -> impl Iterator + '_ { + /// Returns an iterator over retained entries. + pub fn iter(&self) -> impl Iterator> + '_ { self.iter_entries() - .map(|entry| (entry.hash.get(), &entry.summary)) } } diff --git a/datasketches/src/thetafamily/tuple/intersection.rs b/datasketches/src/thetafamily/tuple/intersection.rs index b5e2e437..a38ca05b 100644 --- a/datasketches/src/thetafamily/tuple/intersection.rs +++ b/datasketches/src/thetafamily/tuple/intersection.rs @@ -82,7 +82,7 @@ use crate::tuple::sketch::TupleSketchView; /// /// let result = intersection.to_sketch(true).unwrap(); /// assert_eq!(result.num_retained(), 1); // only "shared" -/// assert_eq!(result.iter().next().unwrap().1, &7); // 3 + 4 +/// assert_eq!(result.iter().next().unwrap().summary(), &7); // 3 + 4 /// ``` #[derive(Debug)] pub struct TupleIntersection

diff --git a/datasketches/src/thetafamily/tuple/mod.rs b/datasketches/src/thetafamily/tuple/mod.rs index 9c3fcf12..c057ee54 100644 --- a/datasketches/src/thetafamily/tuple/mod.rs +++ b/datasketches/src/thetafamily/tuple/mod.rs @@ -50,6 +50,7 @@ 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 e13a462b..22e924f7 100644 --- a/datasketches/src/thetafamily/tuple/sketch.rs +++ b/datasketches/src/thetafamily/tuple/sketch.rs @@ -74,7 +74,7 @@ use crate::tuple::serialization::TupleSummaryValue; /// .unwrap(); /// sketch.update("apple", 1); /// let view = sketch.as_view(); -/// assert_eq!(view.iter().next().unwrap().1, &1); +/// assert_eq!(view.iter().next().unwrap().summary(), &1); /// ``` #[derive(Debug)] pub struct TupleSketchView<'a, S>(TupleSketchViewState<'a, S>); @@ -91,12 +91,12 @@ enum TupleSketchIter<'a, S> { } impl<'a, S> Iterator for TupleSketchIter<'a, S> { - type Item = (u64, &'a S); + type Item = &'a TupleEntry; fn next(&mut self) -> Option { match self { - Self::Mutable(iter) => iter.next().map(|entry| (entry.hash(), entry.summary())), - Self::Compact(iter) => iter.next().map(|entry| (entry.hash(), entry.summary())), + Self::Mutable(iter) => iter.next(), + Self::Compact(iter) => iter.next(), } } @@ -157,8 +157,8 @@ impl<'a, S> TupleSketchView<'a, S> { } } - /// Returns an iterator over retained hashes and borrowed summaries. - pub fn iter(self) -> impl Iterator + 'a { + /// Returns an iterator over retained entries. + pub fn iter(self) -> impl Iterator> + 'a { match self.0 { TupleSketchViewState::Mutable(table) => TupleSketchIter::Mutable(table.iter_entries()), TupleSketchViewState::Compact(sketch) => { @@ -188,7 +188,7 @@ impl KeySketch for TupleSketchView<'_, S> { } fn hashes(self) -> impl Iterator { - self.iter().map(|(hash, _)| hash) + self.iter().map(TupleEntry::hash) } } @@ -199,8 +199,7 @@ where type Entry = TupleEntry; fn entries(self) -> impl Iterator { - self.iter() - .map(|(hash, summary)| TupleEntry::new(hash, summary.clone())) + self.iter().cloned() } } @@ -345,8 +344,8 @@ where self.table.reset(); } - /// Returns an iterator over retained entries as `(hash, &summary)` pairs. - pub fn iter(&self) -> impl Iterator + '_ { + /// Returns an iterator over retained entries. + pub fn iter(&self) -> impl Iterator> + '_ { self.table.iter() } @@ -495,11 +494,9 @@ impl CompactTupleSketch { self.seed_hash } - /// Returns an iterator over retained entries as `(hash, &summary)` pairs. - pub fn iter(&self) -> impl Iterator + '_ { - self.entries - .iter() - .map(|entry| (entry.hash(), entry.summary())) + /// Returns an iterator over retained entries. + pub fn iter(&self) -> impl Iterator> + '_ { + self.entries.iter() } /// Returns the approximate lower error bound given the number of standard deviations. diff --git a/tests-integration/tests/serde_tests/tuple.rs b/tests-integration/tests/serde_tests/tuple.rs index 50dcea90..fe4c7bea 100644 --- a/tests-integration/tests/serde_tests/tuple.rs +++ b/tests-integration/tests/serde_tests/tuple.rs @@ -122,7 +122,7 @@ fn round_trip_preserves_summaries() { CompactTupleSketch::::deserialize(&sketch.compact(true).serialize()).unwrap(); assert_eq!(restored.num_retained(), 50); - let summaries: Vec<_> = restored.iter().map(|(_, &summary)| summary).collect(); + let summaries: Vec<_> = restored.iter().map(|entry| *entry.summary()).collect(); assert_that!(summaries, each(eq(&3))); } diff --git a/tests-integration/tests/tuple_test/a_not_b.rs b/tests-integration/tests/tuple_test/a_not_b.rs index f766136f..397d9576 100644 --- a/tests-integration/tests/tuple_test/a_not_b.rs +++ b/tests-integration/tests/tuple_test/a_not_b.rs @@ -30,7 +30,7 @@ use crate::tuple_sketch_with_range; fn sorted_entries(sketch: &CompactTupleSketch) -> Vec<(u64, u64)> { let mut entries: Vec<_> = sketch .iter() - .map(|(hash, &summary)| (hash, summary)) + .map(|entry| (entry.hash(), *entry.summary())) .collect(); entries.sort_unstable(); entries @@ -49,7 +49,7 @@ fn difference_keeps_only_a_summaries() { assert_eq!(result.num_retained(), 1); assert_eq!(result.estimate(), 1.0); - assert_eq!(result.iter().next().unwrap().1, &5); + assert_eq!(result.iter().next().unwrap().summary(), &5); } #[test] diff --git a/tests-integration/tests/tuple_test/intersection.rs b/tests-integration/tests/tuple_test/intersection.rs index 06cd40b0..9caa0eb8 100644 --- a/tests-integration/tests/tuple_test/intersection.rs +++ b/tests-integration/tests/tuple_test/intersection.rs @@ -77,7 +77,7 @@ fn overlap_combines_summaries() { let result = intersection.to_sketch(true).unwrap(); assert_eq!(result.num_retained(), 1); - assert_eq!(result.iter().next().unwrap().1, &7); + assert_eq!(result.iter().next().unwrap().summary(), &7); } #[test] diff --git a/tests-integration/tests/tuple_test/sketch.rs b/tests-integration/tests/tuple_test/sketch.rs index e48a8973..a4d858f2 100644 --- a/tests-integration/tests/tuple_test/sketch.rs +++ b/tests-integration/tests/tuple_test/sketch.rs @@ -22,6 +22,7 @@ use datasketches::tuple::CompactTupleSketch; use datasketches::tuple::DefaultUpdatePolicy; use datasketches::tuple::SummaryPolicy; use datasketches::tuple::SummaryUpdatePolicy; +use datasketches::tuple::TupleEntry; use datasketches::tuple::TupleSketch; use datasketches::tuple::TupleSketchBuilder; use googletest::assert_that; @@ -60,7 +61,7 @@ fn updates_distinct_keys_and_accumulates_summaries() { assert_eq!(sketch.estimate(), 2.0); assert_eq!(sketch.num_retained(), 2); - let mut summaries: Vec = sketch.iter().map(|(_, &summary)| summary).collect(); + let mut summaries: Vec = sketch.iter().map(|entry| *entry.summary()).collect(); summaries.sort_unstable(); assert_eq!(summaries, [5, 7]); } @@ -86,7 +87,7 @@ fn default_update_policy_accepts_distinct_rhs_type() { sketch.update("key", "hello"); sketch.update("key", " world"); - assert_eq!(sketch.iter().next().unwrap().1, "hello world"); + assert_eq!(sketch.iter().next().unwrap().summary(), "hello world"); } struct ArraySumPolicy { @@ -123,7 +124,10 @@ fn custom_update_policy_accepts_multiple_value_representations() { sketch.update("key", vec![3.0, 4.0]); assert_eq!(sketch.num_retained(), 1); - assert_eq!(sketch.iter().next().unwrap().1.as_slice(), [4.0, 6.0]); + assert_eq!( + sketch.iter().next().unwrap().summary().as_slice(), + [4.0, 6.0] + ); } #[test] @@ -185,8 +189,10 @@ fn empty_sampled_sketch_has_zero_bounds() { assert_eq!(sketch.upper_bound(NumStdDev::Three), 0.0); } -fn sorted_entries<'a>(entries: impl Iterator) -> Vec<(u64, u64)> { - let mut entries: Vec<_> = entries.map(|(hash, &summary)| (hash, summary)).collect(); +fn sorted_entries<'a>(entries: impl Iterator>) -> Vec<(u64, u64)> { + let mut entries: Vec<_> = entries + .map(|entry| (entry.hash(), *entry.summary())) + .collect(); entries.sort_unstable(); entries } diff --git a/tests-integration/tests/tuple_test/union.rs b/tests-integration/tests/tuple_test/union.rs index 3724b081..c29b5426 100644 --- a/tests-integration/tests/tuple_test/union.rs +++ b/tests-integration/tests/tuple_test/union.rs @@ -66,7 +66,7 @@ fn union_combines_overlapping_summaries() { union.update(&b).unwrap(); let result = union.to_sketch(true); - let mut summaries: Vec = result.iter().map(|(_, &summary)| summary).collect(); + let mut summaries: Vec = result.iter().map(|entry| *entry.summary()).collect(); summaries.sort_unstable(); assert_eq!(result.num_retained(), 3); assert_eq!(summaries, [1, 1, 7]); @@ -137,7 +137,7 @@ fn custom_combine_policy_controls_overlapping_summaries() { union.update(&a).unwrap(); union.update(&b).unwrap(); - assert_eq!(union.to_sketch(true).iter().next().unwrap().1, &9); + assert_eq!(union.to_sketch(true).iter().next().unwrap().summary(), &9); } #[test]