Skip to content

Commit b1544aa

Browse files
authored
fix(hll): update silently dropped after deserializing a compact List sketch (#117)
When an `HllSketch` in **List mode** is serialized it uses compact format: only the live coupons are written to disk, with no trailing `COUPON_EMPTY` sentinels. On deserialization, the container was allocated with exactly `coupon_count` slots, leaving the array fully packed. `list::update()` scans linearly for a `COUPON_EMPTY` (`0`) sentinel to find an insertion slot. Finding none, it fell through silently — discarding every value added after the round-trip. Always allocate `1 << lg_arr` slots (initialized to `COUPON_EMPTY`) in `List::deserialize()`, reading only `coupon_count` elements from the byte stream in compact mode. The trailing empty slots are then available as sentinels for subsequent `update()` calls. Fixes #115.
1 parent 7fa974e commit b1544aa

2 files changed

Lines changed: 37 additions & 5 deletions

File tree

datasketches/src/hll/list.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,17 @@ impl List {
8282
empty: bool,
8383
compact: bool,
8484
) -> Result<Self, Error> {
85-
// Compute array size
86-
let array_size = if compact { coupon_count } else { 1 << lg_arr };
87-
88-
// Read coupons
85+
// Always allocate the full-sized array (1 << lg_arr) so Coupon::EMPTY sentinel
86+
// slots are available for future update() calls. In compact format only
87+
// coupon_count values are stored on disk, but memory must hold the full capacity
88+
// so the linear scan in update() can find an empty slot to insert into.
89+
let array_size = 1 << lg_arr;
90+
let read_count = if compact { coupon_count } else { array_size };
91+
92+
// Read coupons into the front of the full-sized array; remaining slots stay Coupon::EMPTY.
8993
let mut coupons = vec![Coupon::EMPTY; array_size];
9094
if !empty && coupon_count > 0 {
91-
for (i, coupon) in coupons.iter_mut().enumerate() {
95+
for (i, coupon) in coupons.iter_mut().take(read_count).enumerate() {
9296
let raw = cursor.read_u32_le().map_err(|_| {
9397
Error::insufficient_data(format!(
9498
"expect {coupon_count} coupons, failed at index {i}"

datasketches/tests/hll_serialization_test.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use std::path::PathBuf;
2222

2323
use common::serialization_test_data;
2424
use datasketches::hll::HllSketch;
25+
use datasketches::hll::HllType;
2526

2627
fn test_sketch_file(path: PathBuf, expected_cardinality: usize, expected_lg_k: u8) {
2728
let expected = expected_cardinality as f64;
@@ -102,6 +103,33 @@ fn test_sketch_file(path: PathBuf, expected_cardinality: usize, expected_lg_k: u
102103
);
103104
}
104105

106+
/// Reproducer for https://github.com/apache/datasketches-rust/issues/115
107+
///
108+
/// A compact-serialized List has no trailing COUPON_EMPTY (0) sentinels.
109+
/// Before the fix, update() would scan the fully-packed array, find no
110+
/// empty slot, and silently drop the new value.
111+
#[test]
112+
fn test_update_after_deserialize_list_mode() {
113+
const LG_K: u8 = 11;
114+
for hll_type in [HllType::Hll4, HllType::Hll6, HllType::Hll8] {
115+
let mut sketch = HllSketch::new(LG_K, hll_type);
116+
sketch.update(1u64);
117+
118+
// Round-trip through serialization (compact format, List mode)
119+
let bytes = sketch.serialize();
120+
let mut sketch = HllSketch::deserialize(&bytes).unwrap();
121+
122+
// This update was silently dropped before the fix
123+
sketch.update(2u64);
124+
125+
let est = sketch.estimate();
126+
assert!(
127+
(est - 2.0).abs() < 0.1,
128+
"{hll_type:?}: expected estimate close to 2.0 after update post-deserialize, got {est}"
129+
);
130+
}
131+
}
132+
105133
#[test]
106134
fn test_java_hll4_compatibility() {
107135
let test_cases = [0, 1, 10, 100, 1000, 10000, 100000, 1000000];

0 commit comments

Comments
 (0)