Skip to content

Commit c9bacae

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 ade8dbe commit c9bacae

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
@@ -585,6 +585,7 @@ mod test {
585585
use vortex_array::IntoArray;
586586
use vortex_array::VortexSessionExecute;
587587
use vortex_array::arrays::BoolArray;
588+
use vortex_array::arrays::Chunked;
588589
use vortex_array::arrays::DecimalArray;
589590
use vortex_array::arrays::FixedSizeListArray;
590591
use vortex_array::arrays::ListArray;
@@ -593,6 +594,7 @@ mod test {
593594
use vortex_array::arrays::StructArray;
594595
use vortex_array::arrays::VarBinArray;
595596
use vortex_array::arrays::VarBinViewArray;
597+
use vortex_array::arrays::chunked::ChunkedArrayExt;
596598
use vortex_array::arrays::listview::ListViewArrayExt;
597599
use vortex_array::arrays::listview::ListViewArraySlotsExt;
598600
use vortex_array::assert_arrays_eq;
@@ -1338,6 +1340,50 @@ mod test {
13381340
Ok(())
13391341
}
13401342

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

330330
use crate::ArrayRef;
331331
use crate::IntoArray;
332+
use crate::RecursiveCanonical;
332333
use crate::VortexSessionExecute;
333334
use crate::aggregate_fn::Accumulator;
334335
use crate::aggregate_fn::AggregateFnVTable;
@@ -368,14 +369,21 @@ mod tests {
368369
use crate::validity::Validity;
369370

370371
fn materialized_uncompressed_size_in_bytes(array: &ArrayRef) -> u64 {
372+
let mut ctx = array_session().create_execution_ctx();
371373
let mut builder = builder_with_capacity(array.dtype(), array.len());
372374
array
373-
.append_to_builder(
374-
builder.as_mut(),
375-
&mut array_session().create_execution_ctx(),
376-
)
375+
.append_to_builder(builder.as_mut(), &mut ctx)
377376
.vortex_expect("appended");
378-
builder.finish().nbytes()
377+
378+
// A builder keeps its children in whatever encoding they were appended in, so the bytes of
379+
// what it finishes only stand in for the uncompressed size once the whole tree is decoded.
380+
builder
381+
.finish()
382+
.execute::<RecursiveCanonical>(&mut ctx)
383+
.vortex_expect("recursively canonicalized")
384+
.0
385+
.into_array()
386+
.nbytes()
379387
}
380388

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

0 commit comments

Comments
 (0)