Skip to content

Commit edb04e4

Browse files
robert3005claude
andcommitted
feat(array): builders no longer canonicalize their children
Nested builders used to push every appended child array through `append_to_builder`, which decoded it into the child's canonical builder. That work is wasted: `Canonical` only promises a canonical *top level*, so struct fields, list elements and extension storage are free to stay compressed. Introduce `ChildBuilder`, which accumulates a child as a `Vec<ArrayRef>` of chunks plus a scalar builder for the values that cannot come from an array, and stitches them into a `ChunkedArray` on `finish` when more than one chunk accumulated. `StructBuilder`, `ListBuilder`, `ListViewBuilder`, `FixedSizeListBuilder` and `ExtensionBuilder` now hold their children this way. However short the appended array, it becomes a chunk. Deciding on the caller's behalf that its values are cheaper copied than referenced would be guessing at a boundary only the caller can see, and a caller that wants them copied has `append_scalar`. A child is therefore chunked on exactly the boundaries it was appended on. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 043d08a commit edb04e4

10 files changed

Lines changed: 994 additions & 49 deletions

File tree

‎encodings/sparse/src/canonical.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,7 @@ mod test {
586586
use vortex_array::IntoArray;
587587
use vortex_array::VortexSessionExecute;
588588
use vortex_array::arrays::BoolArray;
589+
use vortex_array::arrays::Chunked;
589590
use vortex_array::arrays::DecimalArray;
590591
use vortex_array::arrays::FixedSizeListArray;
591592
use vortex_array::arrays::ListArray;
@@ -594,6 +595,7 @@ mod test {
594595
use vortex_array::arrays::StructArray;
595596
use vortex_array::arrays::VarBinArray;
596597
use vortex_array::arrays::VarBinViewArray;
598+
use vortex_array::arrays::chunked::ChunkedArrayExt;
597599
use vortex_array::arrays::listview::ListViewArrayExt;
598600
use vortex_array::arrays::listview::ListViewArraySlotsExt;
599601
use vortex_array::assert_arrays_eq;
@@ -1339,6 +1341,50 @@ mod test {
13391341
Ok(())
13401342
}
13411343

1344+
/// Nested builders chunk a child on the boundaries it is appended on, so the number of appends
1345+
/// canonicalization makes is now visible in the elements child. A run of consecutive patches
1346+
/// and the gap after it should cost one chunk each, not one chunk per row.
1347+
#[test]
1348+
fn test_sparse_list_chunks_elements_per_run_not_per_patch() -> VortexResult<()> {
1349+
let mut ctx = SESSION.create_execution_ctx();
1350+
1351+
const PATCHES: usize = 100;
1352+
let patches_u32 = u32::try_from(PATCHES).vortex_expect("fits in u32");
1353+
let patches_i32 = i32::try_from(PATCHES).vortex_expect("fits in i32");
1354+
1355+
// `PATCHES` single-element lists, patched onto rows 0..PATCHES of a 2 * PATCHES-row array,
1356+
// so there is exactly one patch run followed by exactly one gap.
1357+
let patch_values = ListViewArray::new(
1358+
PrimitiveArray::from_iter(0..patches_i32).into_array(),
1359+
PrimitiveArray::from_iter(0..patches_u32).into_array(),
1360+
PrimitiveArray::from_iter(std::iter::repeat_n(1u32, PATCHES)).into_array(),
1361+
Validity::AllValid,
1362+
)
1363+
.into_array();
1364+
1365+
let indices = PrimitiveArray::from_iter(0..patches_u32).into_array();
1366+
let fill = Scalar::from(Some(vec![-1i32]));
1367+
let sparse = Sparse::try_new(indices, patch_values, 2 * PATCHES, fill)?.into_array();
1368+
1369+
let actual = sparse.execute::<ListViewArray>(&mut ctx)?;
1370+
assert_eq!(
1371+
actual.elements().as_::<Chunked>().nchunks(),
1372+
2,
1373+
"expected one chunk for the patch run and one for the gap",
1374+
);
1375+
1376+
let expected_lists = (0..patches_i32)
1377+
.map(|i| Some(vec![i]))
1378+
.chain(std::iter::repeat_n(Some(vec![-1i32]), PATCHES));
1379+
let expected = ListArray::from_iter_opt_slow::<u32, _, _>(
1380+
expected_lists,
1381+
Arc::new(PType::I32.into()),
1382+
)?;
1383+
assert_arrays_eq!(actual, expected, &mut ctx);
1384+
1385+
Ok(())
1386+
}
1387+
13421388
/// Patches on consecutive rows are appended as one slice of the patch array, so this covers the
13431389
/// run arithmetic together with everything that has to survive it: a null patch inside a run,
13441390
/// patch views that overlap and are out of order, runs separated by gaps, and a trailing gap.

‎vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@ mod tests {
341341

342342
use crate::ArrayRef;
343343
use crate::IntoArray;
344+
use crate::RecursiveCanonical;
344345
use crate::VortexSessionExecute;
345346
use crate::aggregate_fn::Accumulator;
346347
use crate::aggregate_fn::AggregateFnVTable;
@@ -380,14 +381,21 @@ mod tests {
380381
use crate::validity::Validity;
381382

382383
fn materialized_uncompressed_size_in_bytes(array: &ArrayRef) -> u64 {
384+
let mut ctx = array_session().create_execution_ctx();
383385
let mut builder = builder_with_capacity(array.dtype(), array.len());
384386
array
385-
.append_to_builder(
386-
builder.as_mut(),
387-
&mut array_session().create_execution_ctx(),
388-
)
387+
.append_to_builder(builder.as_mut(), &mut ctx)
389388
.vortex_expect("appended");
390-
builder.finish().nbytes()
389+
390+
// A builder keeps its children in whatever encoding they were appended in, so the bytes of
391+
// what it finishes only stand in for the uncompressed size once the whole tree is decoded.
392+
builder
393+
.finish()
394+
.execute::<RecursiveCanonical>(&mut ctx)
395+
.vortex_expect("recursively canonicalized")
396+
.0
397+
.into_array()
398+
.nbytes()
391399
}
392400

393401
fn aggregate(array: &ArrayRef) -> VortexResult<u64> {

0 commit comments

Comments
 (0)