Skip to content

Commit e9cfe55

Browse files
authored
fix(hll): preserve set coupon order when serializing (#128)
* fix(hll): preserve set coupon order when serializing Compact HLL Set serialization used to sort retained coupons before writing bytes. Java and C++ write compact Set coupons in hash table iteration order. Write the existing iterator output directly. This keeps the compact format and removes the extra allocation. * test(hll): add byte exact coupon serialization checks
1 parent 9a80c60 commit e9cfe55

2 files changed

Lines changed: 57 additions & 11 deletions

File tree

datasketches/src/hll/hash_set.rs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -179,17 +179,7 @@ impl HashSet {
179179

180180
// Write coupons
181181
if compact {
182-
// Compact mode: collect non-empty coupons and sort for deterministic output
183-
let mut coupons_vec: Vec<Coupon> = self
184-
.container
185-
.coupons
186-
.iter()
187-
.filter(|&&c| !c.is_empty())
188-
.copied()
189-
.collect();
190-
coupons_vec.sort_unstable();
191-
192-
for coupon in coupons_vec.iter().copied() {
182+
for coupon in self.container.iter() {
193183
bytes.write_u32_le(coupon.raw());
194184
}
195185
} else {

datasketches/tests/hll_serialization_test.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use std::fs;
2323
use std::path::PathBuf;
2424

2525
use common::serialization_test_data;
26+
use datasketches::hash_value::natural_extend;
2627
use datasketches::hll::HllSketch;
2728
use datasketches::hll::HllType;
2829

@@ -132,6 +133,61 @@ fn test_update_after_deserialize_list_mode() {
132133
}
133134
}
134135

136+
#[test]
137+
fn test_serialized_bytes_match_reference_files_for_coupon_modes() {
138+
fn serialized_mode_name(bytes: &[u8]) -> &'static str {
139+
// The HLL preamble stores current mode in the low two bits of byte 7.
140+
match bytes[7] & 0x3 {
141+
0 => "List",
142+
1 => "Set",
143+
2 => "HLL",
144+
_ => "unknown",
145+
}
146+
}
147+
148+
for (hll_type, type_name) in [
149+
(HllType::Hll4, "hll4"),
150+
(HllType::Hll6, "hll6"),
151+
(HllType::Hll8, "hll8"),
152+
] {
153+
for (n, mode) in [(0_u32, "List"), (1, "List"), (10, "Set"), (100, "Set")] {
154+
// Fixture generators use lg_k 12 and update the sketch with 0..n.
155+
let mut sketch = HllSketch::new(12, hll_type);
156+
for value in 0..n {
157+
sketch.update(natural_extend::from_u32(value));
158+
}
159+
160+
let bytes = sketch.serialize();
161+
assert_eq!(
162+
serialized_mode_name(&bytes),
163+
mode,
164+
"Rust {type_name} n{n} should serialize in {mode} mode"
165+
);
166+
167+
for (dir, suffix) in [
168+
("java_generated_files", "java"),
169+
("cpp_generated_files", "cpp"),
170+
] {
171+
let filename = format!("{type_name}_n{n}_{suffix}.sk");
172+
let path = serialization_test_data(dir, &filename);
173+
let expected = fs::read(&path).unwrap();
174+
assert_eq!(
175+
serialized_mode_name(&expected),
176+
mode,
177+
"{} should be a {mode} mode fixture",
178+
path.display()
179+
);
180+
assert_eq!(
181+
bytes,
182+
expected,
183+
"Rust {type_name} n{n} {mode} bytes must match {}",
184+
path.display()
185+
);
186+
}
187+
}
188+
}
189+
}
190+
135191
#[test]
136192
fn test_java_hll4_compatibility() {
137193
let test_cases = [0, 1, 10, 100, 1000, 10000, 100000, 1000000];

0 commit comments

Comments
 (0)