Skip to content

Commit 1e93ddc

Browse files
committed
streaming_chunker: don't emit boundary on internal-level chunks with <2 entries
The Python CI hit a segfault (release) / u8 overflow panic (debug) in basic_usage.py at the file-backed `insert` step. Root cause: Python's `TreeConfig.__init__` has `pattern=0` as its default, and the example also sets `min_chunk_size=1`. With pattern=0 the splitter's `(hash & pattern) == pattern` check is true on every item, so a boundary fires on every append. The chunker's `handle_boundary` then recursively created a new parent chunker one level up, whose splitter saw the just-emitted `(firstKey, hash)` entry, fired a boundary, and cascaded - climbing the `u8` level counter until it overflowed. This is Dolt's "constraint (3): internal nodes must contain at least 2 key-value pairs" rule from `chunker.go::append`, which we didn't port. Adding it: at level > 0, defer boundary emission until the in-progress chunk has at least 2 entries. Leaf-level chunks (level 0) are still allowed to be single-entry so we can store a 1-key dataset. The old in-place `Balanced::balance` didn't have this bug because it emitted upward only when the chunker reported >1 chunks (its `chunks.len() <= 1` early-return). The streaming chunker emits at every boundary, so we need the explicit constraint here. Regression test in `streaming_chunker::tests` reproduces the degenerate config and asserts the level stays bounded. Verified by re-running basic_usage.py against a debug + release build, both succeed end-to-end. Rust suite: 160 lib tests pass, 0 failed.
1 parent 66cf674 commit 1e93ddc

1 file changed

Lines changed: 48 additions & 1 deletion

File tree

src/streaming_chunker.rs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,18 @@ impl<'s, const N: usize, S: NodeStorage<N>> Chunker<'s, N, S> {
352352
self.splitter.append(&key, &value);
353353
self.builder.add(key, value);
354354

355-
if self.splitter.crossed_boundary() {
355+
// Dolt's "constraint (3)": an internal-level chunk must contain
356+
// at least 2 entries. A single-entry internal node would be a
357+
// degenerate tower - emitting a boundary at level L on one
358+
// entry creates a level-(L+1) chunker that also sees one entry
359+
// and (with the same splitter config) also fires a boundary,
360+
// cascading infinitely. Defer emission until the in-progress
361+
// chunk has at least 2 entries. At level 0 (leaves), single-
362+
// entry chunks are allowed because the alternative would be no
363+
// tree at all for a 1-key dataset.
364+
let degenerate_internal = self.level > 0 && self.builder.count() < 2;
365+
366+
if self.splitter.crossed_boundary() && !degenerate_internal {
356367
self.handle_boundary(storage);
357368
}
358369
}
@@ -1144,4 +1155,40 @@ mod tests {
11441155

11451156
assert_eq!(result.get_hash(), expected.get_hash());
11461157
}
1158+
1159+
/// Regression test for the "every-item boundary" degenerate config:
1160+
/// `pattern = 0` makes `(hash & pattern) == pattern` true for every
1161+
/// item, so the splitter fires a boundary on every append. Combined
1162+
/// with `min_chunk_size = 1` (Python's `TreeConfig.__init__` default
1163+
/// at the time of writing), this used to cascade through the
1164+
/// chunker's recursive parent creation until the level counter
1165+
/// (`u8`) overflowed - panicking in debug, segfaulting in release.
1166+
/// The "constraint (3)" check in `Chunker::append` (skip emission
1167+
/// when an internal-level builder has fewer than 2 entries) breaks
1168+
/// the cascade.
1169+
#[test]
1170+
fn degenerate_pattern_zero_does_not_cascade() {
1171+
let cfg = TreeConfig::<32> {
1172+
base: 4,
1173+
modulus: 64,
1174+
min_chunk_size: 1,
1175+
max_chunk_size: 4096,
1176+
pattern: 0,
1177+
root_hash: None,
1178+
key_schema: None,
1179+
value_schema: None,
1180+
encode_types: vec![],
1181+
};
1182+
let mut storage = InMemoryNodeStorage::<32>::default();
1183+
// Stream a handful of items through; this used to overflow the
1184+
// u8 level counter via recursive `handle_boundary` -> `append`.
1185+
let root = build_tree_from_sorted_pairs::<32, _>(
1186+
(0u64..5).map(|i| (key(i), val(i))),
1187+
&cfg,
1188+
&mut storage,
1189+
);
1190+
// Sanity: tree should hold all 5 items, and its level should be
1191+
// small (definitely not approaching u8::MAX).
1192+
assert!(root.level < 16, "tree level grew too tall: {}", root.level);
1193+
}
11471194
}

0 commit comments

Comments
 (0)