From ed83ff0b8234c0245477b1b705a0fe19b8248431 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 11:45:21 +0000 Subject: [PATCH 1/4] Make `ArrayBuilder::extend_from_array{,_unchecked}` return `VortexResult` Builders are fallible because array canonicalization/execute is fallible. The `extend_from_array` and `extend_from_array_unchecked` methods previously swallowed these failures via `vortex_panic!`/`vortex_expect`. Change both methods to return `VortexResult<()>` and propagate errors instead of panicking. Rather than only switching the signatures, the panic/expect sites inside the extend implementations that stem from fallible operations are converted to error propagation: - canonicalization now goes through `execute::(ctx)?` instead of the deprecated, panicking `to_*()` `ToCanonical` helpers, - validity computation (`validity()?`, `execute_mask(..)?`), `rebuild`, `cast`, and `slice` failures are propagated with `?`, - the trait's dtype-mismatch guard uses `vortex_bail!` instead of `vortex_panic!`. Infallible "impossible" integer offset conversions are left as `vortex_expect`. All call sites are updated to propagate or `unwrap` the result. This is a breaking change to the (mostly internal) builder API. Signed-off-by: Joseph Isaacs --- encodings/experimental/onpair/src/array.rs | 2 +- .../src/bitpacking/array/bitpack_compress.rs | 2 +- encodings/fsst/src/array.rs | 2 +- .../benches/listview_builder_extend.rs | 9 +++- .../fns/uncompressed_size_in_bytes/mod.rs | 2 +- vortex-array/src/array/vtable/mod.rs | 2 +- .../src/arrays/constant/vtable/mod.rs | 2 +- vortex-array/src/arrays/listview/rebuild.rs | 2 +- vortex-array/src/arrays/patched/vtable/mod.rs | 2 +- vortex-array/src/arrays/varbinview/compact.rs | 2 +- vortex-array/src/builders/bool.rs | 20 +++----- vortex-array/src/builders/decimal.rs | 21 +++----- vortex-array/src/builders/extension.rs | 10 ++-- vortex-array/src/builders/fixed_size_list.rs | 37 ++++++-------- vortex-array/src/builders/list.rs | 50 +++++++++---------- vortex-array/src/builders/listview.rs | 49 ++++++++---------- vortex-array/src/builders/mod.rs | 14 ++++-- vortex-array/src/builders/null.rs | 3 +- vortex-array/src/builders/primitive.rs | 20 +++----- vortex-array/src/builders/struct_.rs | 21 +++----- vortex-array/src/builders/varbinview.rs | 18 +++---- 21 files changed, 131 insertions(+), 159 deletions(-) diff --git a/encodings/experimental/onpair/src/array.rs b/encodings/experimental/onpair/src/array.rs index f9e18c36ac4..e576af92c11 100644 --- a/encodings/experimental/onpair/src/array.rs +++ b/encodings/experimental/onpair/src/array.rs @@ -482,7 +482,7 @@ impl VTable for OnPair { .clone() .execute::(ctx)? .into_array(), - ); + )?; return Ok(()); }; diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs index eba5be1b749..f8c290cee75 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs @@ -519,7 +519,7 @@ mod test { let mut primitive_builder = PrimitiveBuilder::::with_capacity(chunked.dtype().nullability(), 10 * 100); - primitive_builder.extend_from_array(&chunked); + primitive_builder.extend_from_array(&chunked)?; let ca_into = primitive_builder.finish(); assert_arrays_eq!(into_ca, ca_into); diff --git a/encodings/fsst/src/array.rs b/encodings/fsst/src/array.rs index 05a07b34c4d..8315cda0c9c 100644 --- a/encodings/fsst/src/array.rs +++ b/encodings/fsst/src/array.rs @@ -292,7 +292,7 @@ impl VTable for FSST { .clone() .execute::(ctx)? .into_array(), - ); + )?; return Ok(()); }; diff --git a/vortex-array/benches/listview_builder_extend.rs b/vortex-array/benches/listview_builder_extend.rs index 75487564ea8..8f686e6616b 100644 --- a/vortex-array/benches/listview_builder_extend.rs +++ b/vortex-array/benches/listview_builder_extend.rs @@ -18,6 +18,7 @@ use vortex_array::dtype::Nullability::Nullable; use vortex_array::dtype::PType::I32; use vortex_array::validity::Validity; use vortex_buffer::Buffer; +use vortex_error::VortexExpect; fn main() { divan::main(); @@ -69,7 +70,9 @@ fn extend_from_array_zctl(bencher: Bencher, (num_lists, list_size): (usize, usiz num_lists * list_size, num_lists, ); - builder.extend_from_array(source); + builder + .extend_from_array(source) + .vortex_expect("extend_from_array"); divan::black_box(builder.finish_into_listview()) }); } @@ -91,7 +94,9 @@ fn extend_from_array_non_zctl_overlapping( num_lists * list_size, num_lists, ); - builder.extend_from_array(source); + builder + .extend_from_array(source) + .vortex_expect("extend_from_array"); divan::black_box(builder.finish_into_listview()) }); } diff --git a/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs b/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs index 7a77ed9555c..d83a060edc0 100644 --- a/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs @@ -345,7 +345,7 @@ mod tests { fn materialized_uncompressed_size_in_bytes(array: &ArrayRef) -> u64 { let mut builder = builder_with_capacity(array.dtype(), array.len()); unsafe { - builder.extend_from_array_unchecked(array); + builder.extend_from_array_unchecked(array).unwrap(); } builder.finish().nbytes() } diff --git a/vortex-array/src/array/vtable/mod.rs b/vortex-array/src/array/vtable/mod.rs index 366eed77ad9..54941940da1 100644 --- a/vortex-array/src/array/vtable/mod.rs +++ b/vortex-array/src/array/vtable/mod.rs @@ -151,7 +151,7 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug { .clone() .execute::(ctx)? .into_array(); - builder.extend_from_array(&canonical); + builder.extend_from_array(&canonical)?; Ok(()) } diff --git a/vortex-array/src/arrays/constant/vtable/mod.rs b/vortex-array/src/arrays/constant/vtable/mod.rs index 89c571eb827..fd32de8de76 100644 --- a/vortex-array/src/arrays/constant/vtable/mod.rs +++ b/vortex-array/src/arrays/constant/vtable/mod.rs @@ -236,7 +236,7 @@ impl VTable for Constant { .clone() .execute::(ctx)? .into_array(); - builder.extend_from_array(&canonical); + builder.extend_from_array(&canonical)?; } } diff --git a/vortex-array/src/arrays/listview/rebuild.rs b/vortex-array/src/arrays/listview/rebuild.rs index c42f1bf85a1..1fe17e01cc2 100644 --- a/vortex-array/src/arrays/listview/rebuild.rs +++ b/vortex-array/src/arrays/listview/rebuild.rs @@ -309,7 +309,7 @@ impl ListViewArray { new_offsets.push(n_elements); new_sizes.push(size); - new_elements_builder.extend_from_array(&elements_canonical.slice(start..stop)?); + new_elements_builder.extend_from_array(&elements_canonical.slice(start..stop)?)?; n_elements += num_traits::cast(size).vortex_expect("Cast failed"); } diff --git a/vortex-array/src/arrays/patched/vtable/mod.rs b/vortex-array/src/arrays/patched/vtable/mod.rs index 4ba5bc542e1..1bdd2a37a60 100644 --- a/vortex-array/src/arrays/patched/vtable/mod.rs +++ b/vortex-array/src/arrays/patched/vtable/mod.rs @@ -198,7 +198,7 @@ impl VTable for Patched { .clone() .execute::(ctx)? .into_array(); - builder.extend_from_array(&canonical); + builder.extend_from_array(&canonical)?; return Ok(()); } diff --git a/vortex-array/src/arrays/varbinview/compact.rs b/vortex-array/src/arrays/varbinview/compact.rs index 6effc7c656a..3c96d8886a2 100644 --- a/vortex-array/src/arrays/varbinview/compact.rs +++ b/vortex-array/src/arrays/varbinview/compact.rs @@ -136,7 +136,7 @@ impl VarBinViewArray { self.len(), buffer_utilization_threshold, ); - builder.extend_from_array(&self.clone().into_array()); + builder.extend_from_array(&self.clone().into_array())?; Ok(builder.finish_into_varbinview()) } } diff --git a/vortex-array/src/builders/bool.rs b/vortex-array/src/builders/bool.rs index fdae7984844..a5a56038c59 100644 --- a/vortex-array/src/builders/bool.rs +++ b/vortex-array/src/builders/bool.rs @@ -5,7 +5,6 @@ use std::any::Any; use std::mem; use vortex_buffer::BitBufferMut; -use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_ensure; use vortex_mask::Mask; @@ -20,8 +19,6 @@ use crate::builders::ArrayBuilder; use crate::builders::DEFAULT_BUILDER_CAPACITY; use crate::builders::LazyBitBufferBuilder; use crate::canonical::Canonical; -#[expect(deprecated)] -use crate::canonical::ToCanonical as _; use crate::dtype::DType; use crate::dtype::Nullability; use crate::scalar::Scalar; @@ -115,22 +112,19 @@ impl ArrayBuilder for BoolBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let bool_array = array.to_bool(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let bool_array = array.clone().execute::(&mut ctx)?; self.inner.append_buffer(&bool_array.to_bit_buffer()); self.nulls.append_validity_mask( &bool_array .as_ref() - .validity() - .vortex_expect("validity_mask") - .execute_mask( - bool_array.as_ref().len(), - &mut LEGACY_SESSION.create_execution_ctx(), - ) - .vortex_expect("Failed to compute validity mask"), + .validity()? + .execute_mask(bool_array.as_ref().len(), &mut ctx)?, ); + + Ok(()) } fn reserve_exact(&mut self, additional: usize) { diff --git a/vortex-array/src/builders/decimal.rs b/vortex-array/src/builders/decimal.rs index f8d438d477b..351a7e078d2 100644 --- a/vortex-array/src/builders/decimal.rs +++ b/vortex-array/src/builders/decimal.rs @@ -14,8 +14,6 @@ use vortex_mask::Mask; use crate::ArrayRef; use crate::IntoArray; use crate::LEGACY_SESSION; -#[expect(deprecated)] -use crate::ToCanonical as _; use crate::VortexSessionExecute; use crate::arrays::DecimalArray; use crate::builders::ArrayBuilder; @@ -195,9 +193,9 @@ impl ArrayBuilder for DecimalBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let decimal_array = array.to_decimal(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let decimal_array = array.clone().execute::(&mut ctx)?; match_each_decimal_value_type!(decimal_array.values_type(), |D| { // Extends the values buffer from another buffer of type D where D can be coerced to the @@ -209,14 +207,11 @@ impl ArrayBuilder for DecimalBuilder { self.nulls.append_validity_mask( &decimal_array .as_ref() - .validity() - .vortex_expect("validity_mask") - .execute_mask( - decimal_array.as_ref().len(), - &mut LEGACY_SESSION.create_execution_ctx(), - ) - .vortex_expect("Failed to compute validity mask"), + .validity()? + .execute_mask(decimal_array.as_ref().len(), &mut ctx)?, ); + + Ok(()) } fn reserve_exact(&mut self, additional: usize) { @@ -328,7 +323,7 @@ mod tests { let i8s = i8s.finish(); let mut i128s = DecimalBuilder::new::(DecimalDType::new(2, 1), false.into()); - i128s.extend_from_array(&i8s); + i128s.extend_from_array(&i8s).unwrap(); let i128s = i128s.finish(); for i in 0..i8s.len() { diff --git a/vortex-array/src/builders/extension.rs b/vortex-array/src/builders/extension.rs index d947877e297..d079a6375df 100644 --- a/vortex-array/src/builders/extension.rs +++ b/vortex-array/src/builders/extension.rs @@ -9,14 +9,14 @@ use vortex_mask::Mask; use crate::ArrayRef; use crate::IntoArray; +use crate::LEGACY_SESSION; +use crate::VortexSessionExecute; use crate::arrays::ExtensionArray; use crate::arrays::extension::ExtensionArrayExt; use crate::builders::ArrayBuilder; use crate::builders::DEFAULT_BUILDER_CAPACITY; use crate::builders::builder_with_capacity; use crate::canonical::Canonical; -#[expect(deprecated)] -use crate::canonical::ToCanonical as _; use crate::dtype::DType; use crate::dtype::extension::ExtDTypeRef; use crate::scalar::ExtScalar; @@ -99,9 +99,9 @@ impl ArrayBuilder for ExtensionBuilder { self.append_value(scalar.as_extension()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let ext_array = array.to_extension(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let ext_array = array.clone().execute::(&mut ctx)?; self.storage.extend_from_array(ext_array.storage_array()) } diff --git a/vortex-array/src/builders/fixed_size_list.rs b/vortex-array/src/builders/fixed_size_list.rs index aece8c21465..174c1c80178 100644 --- a/vortex-array/src/builders/fixed_size_list.rs +++ b/vortex-array/src/builders/fixed_size_list.rs @@ -22,8 +22,6 @@ use crate::builders::DEFAULT_BUILDER_CAPACITY; use crate::builders::LazyBitBufferBuilder; use crate::builders::builder_with_capacity; use crate::canonical::Canonical; -#[expect(deprecated)] -use crate::canonical::ToCanonical as _; use crate::dtype::DType; use crate::dtype::Nullability; use crate::scalar::ListScalar; @@ -97,7 +95,7 @@ impl FixedSizeListBuilder { self.list_size() ); - self.elements_builder.extend_from_array(array); + self.elements_builder.extend_from_array(array)?; self.nulls.append_non_null(); Ok(()) @@ -237,21 +235,18 @@ impl ArrayBuilder for FixedSizeListBuilder { /// This will increase the capacity if extending with this `array` would go past the original /// capacity. - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let fsl = array.to_fixed_size_list(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let fsl = array.clone().execute::(&mut ctx)?; if fsl.is_empty() { - return; + return Ok(()); } - self.elements_builder.extend_from_array(fsl.elements()); - self.nulls.append_validity_mask( - &array - .validity() - .vortex_expect("validity_mask in extend_from_array_unchecked") - .execute_mask(array.len(), &mut LEGACY_SESSION.create_execution_ctx()) - .vortex_expect("Failed to compute validity mask"), - ); + self.elements_builder.extend_from_array(fsl.elements())?; + self.nulls + .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); + + Ok(()) } fn reserve_exact(&mut self, additional: usize) { @@ -675,8 +670,8 @@ mod tests { let mut builder = FixedSizeListBuilder::with_capacity(dtype, 2, Nullable, 0); let source_array = source.into_array(); - builder.extend_from_array(&source_array); - builder.extend_from_array(&source_array); + builder.extend_from_array(&source_array).unwrap(); + builder.extend_from_array(&source_array).unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 6); @@ -751,8 +746,8 @@ mod tests { let mut builder = FixedSizeListBuilder::with_capacity(dtype, 0, Nullable, 0); - builder.extend_from_array(&source1.into_array()); - builder.extend_from_array(&source2.into_array()); + builder.extend_from_array(&source1.into_array()).unwrap(); + builder.extend_from_array(&source2.into_array()).unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 5); @@ -828,7 +823,7 @@ mod tests { .unwrap(); // Extend with empty array (should be no-op). - builder.extend_from_array(&source.into_array()); + builder.extend_from_array(&source.into_array()).unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 1); @@ -865,7 +860,7 @@ mod tests { Validity::AllValid, 1, ); - builder.extend_from_array(&source.into_array()); + builder.extend_from_array(&source.into_array()).unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 6); diff --git a/vortex-array/src/builders/list.rs b/vortex-array/src/builders/list.rs index a0c1fdd581b..69584a81901 100644 --- a/vortex-array/src/builders/list.rs +++ b/vortex-array/src/builders/list.rs @@ -17,6 +17,8 @@ use crate::IntoArray; use crate::LEGACY_SESSION; use crate::VortexSessionExecute; use crate::arrays::ListArray; +use crate::arrays::ListViewArray; +use crate::arrays::PrimitiveArray; use crate::arrays::listview::ListViewArrayExt; use crate::builders::ArrayBuilder; use crate::builders::DEFAULT_BUILDER_CAPACITY; @@ -103,7 +105,7 @@ impl ListBuilder { self.element_dtype() ); - self.elements_builder.extend_from_array(array); + self.elements_builder.extend_from_array(array)?; self.nulls.append_non_null(); self.offsets_builder.append_value( O::from_usize(self.elements_builder.len()) @@ -217,35 +219,29 @@ impl ArrayBuilder for ListBuilder { self.append_value(scalar.as_list()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let list = array.to_listview(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let list = array.clone().execute::(&mut ctx)?; if list.is_empty() { - return; + return Ok(()); } // Append validity information. - self.nulls.append_validity_mask( - &array - .validity() - .vortex_expect("validity_mask in extend_from_array_unchecked") - .execute_mask(array.len(), &mut LEGACY_SESSION.create_execution_ctx()) - .vortex_expect("Failed to compute validity mask"), - ); + self.nulls + .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); // Note that `ListViewArray` has `n` offsets and sizes, not `n+1` offsets like `ListArray`. let elements = list.elements(); - #[expect(deprecated)] - let offsets = list.offsets().to_primitive(); - #[expect(deprecated)] - let sizes = list.sizes().to_primitive(); + let offsets = list.offsets().clone().execute::(&mut ctx)?; + let sizes = list.sizes().clone().execute::(&mut ctx)?; fn extend_inner( builder: &mut ListBuilder, new_elements: &ArrayRef, new_offsets: &[OffsetType], new_sizes: &[SizeType], - ) where + ) -> VortexResult<()> + where O: IntegerPType, OffsetType: IntegerPType, SizeType: IntegerPType, @@ -263,10 +259,8 @@ impl ArrayBuilder for ListBuilder { let size: usize = new_sizes[i].as_(); if size > 0 { - let list_elements = new_elements - .slice(offset..offset + size) - .vortex_expect("list builder slice"); - builder.elements_builder.extend_from_array(&list_elements); + let list_elements = new_elements.slice(offset..offset + size)?; + builder.elements_builder.extend_from_array(&list_elements)?; curr_offset += size; } @@ -279,6 +273,8 @@ impl ArrayBuilder for ListBuilder { // SAFETY: We have initialized all `num_lists` values, and since the `offsets` array is // non-nullable, we are done. unsafe { offsets_range.finish() }; + + Ok(()) } match_each_integer_ptype!(offsets.ptype(), |OffsetType| { @@ -454,10 +450,14 @@ mod tests { let mut ctx = LEGACY_SESSION.create_execution_ctx(); let mut builder = ListBuilder::::with_capacity(Arc::new(I32.into()), Nullable, 18, 9); - builder.extend_from_array(&list); - builder.extend_from_array(&list); - builder.extend_from_array(&list.slice(0..0).unwrap()); - builder.extend_from_array(&list.slice(1..3).unwrap()); + builder.extend_from_array(&list).unwrap(); + builder.extend_from_array(&list).unwrap(); + builder + .extend_from_array(&list.slice(0..0).unwrap()) + .unwrap(); + builder + .extend_from_array(&list.slice(1..3).unwrap()) + .unwrap(); #[expect(deprecated)] let expected = ListArray::from_iter_opt_slow::( diff --git a/vortex-array/src/builders/listview.rs b/vortex-array/src/builders/listview.rs index eeae7886026..84109cf8dbb 100644 --- a/vortex-array/src/builders/listview.rs +++ b/vortex-array/src/builders/listview.rs @@ -135,7 +135,7 @@ impl ListViewBuilder { "appending this list would cause an offset overflow" ); - self.elements_builder.extend_from_array(array); + self.elements_builder.extend_from_array(array)?; self.nulls.append_non_null(); self.offsets_builder.append_value( @@ -291,41 +291,32 @@ impl ArrayBuilder for ListViewBuilder { self.append_value(list_scalar) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { // TODO: The `ArrayBuilder` trait does not thread an `ExecutionCtx` through its extend // methods, so we are forced to mint a fresh `LEGACY_SESSION` context here on every call // (which for chunked input means once per chunk). Once the trait carries a `&mut // ExecutionCtx`, the caller's session should be reused instead. let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let listview = array - .clone() - .execute::(&mut ctx) - .vortex_expect("failed to execute array into ListViewArray in extend_from_array"); + let listview = array.clone().execute::(&mut ctx)?; if listview.is_empty() { - return; + return Ok(()); } // Normalize to an exact zero-copy-to-list layout and then bulk append. This avoids the // very expensive scalar_at-per-list path for overlapping / out-of-order list views. - let listview = listview - .rebuild(ListViewRebuildMode::MakeExact, &mut ctx) - .vortex_expect("ListViewArray::rebuild(MakeExact) failed in extend_from_array"); + let listview = listview.rebuild(ListViewRebuildMode::MakeExact, &mut ctx)?; debug_assert!(listview.is_zero_copy_to_list()); - self.nulls.append_validity_mask( - &array - .validity() - .vortex_expect("validity_mask in extend_from_array_unchecked") - .execute_mask(array.len(), &mut ctx) - .vortex_expect("Failed to compute validity mask"), - ); + self.nulls + .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); // Bulk append the new elements (which should have no gaps or overlaps). let old_elements_len = self.elements_builder.len(); self.elements_builder .reserve_exact(listview.elements().len()); - self.elements_builder.extend_from_array(listview.elements()); + self.elements_builder + .extend_from_array(listview.elements())?; let new_elements_len = self.elements_builder.len(); // Reserve enough space for the new views. @@ -337,11 +328,8 @@ impl ArrayBuilder for ListViewBuilder { let cast_sizes = listview .sizes() .clone() - .cast(self.sizes_builder.dtype().clone()) - .vortex_expect( - "was somehow unable to cast the new sizes to the type of the builder sizes", - ); - self.sizes_builder.extend_from_array(&cast_sizes); + .cast(self.sizes_builder.dtype().clone())?; + self.sizes_builder.extend_from_array(&cast_sizes)?; // Now we need to adjust all of the offsets by adding the current number of elements in the // builder. @@ -352,8 +340,7 @@ impl ArrayBuilder for ListViewBuilder { let new_offsets = listview .offsets() .clone() - .execute::(&mut ctx) - .vortex_expect("failed to execute list view offsets into a PrimitiveArray"); + .execute::(&mut ctx)?; match_each_integer_ptype!(new_offsets.ptype(), |A| { adjust_and_extend_offsets::( @@ -362,7 +349,9 @@ impl ArrayBuilder for ListViewBuilder { old_elements_len, new_elements_len, ); - }) + }); + + Ok(()) } fn reserve_exact(&mut self, capacity: usize) { @@ -657,7 +646,7 @@ mod tests { .unwrap(); // Extend from the ListArray. - builder.extend_from_array(&source.into_array()); + builder.extend_from_array(&source.into_array()).unwrap(); // Extend from empty array (should be no-op). let empty_source = ListArray::from_iter_opt_slow::>( @@ -665,7 +654,9 @@ mod tests { Arc::new(I32.into()), ) .unwrap(); - builder.extend_from_array(&empty_source.into_array()); + builder + .extend_from_array(&empty_source.into_array()) + .unwrap(); let listview = builder.finish_into_listview(); assert_eq!(listview.len(), 4); @@ -719,7 +710,7 @@ mod tests { let mut builder = ListViewBuilder::::with_capacity(Arc::clone(&dtype), Nullable, 0, 0); - builder.extend_from_array(&source.into_array()); + builder.extend_from_array(&source.into_array()).unwrap(); let listview = builder.finish_into_listview(); assert_eq!(listview.len(), 3); diff --git a/vortex-array/src/builders/mod.rs b/vortex-array/src/builders/mod.rs index e9f86fa4332..39e39d94b8d 100644 --- a/vortex-array/src/builders/mod.rs +++ b/vortex-array/src/builders/mod.rs @@ -34,7 +34,7 @@ use std::any::Any; use std::sync::Arc; use vortex_error::VortexResult; -use vortex_error::vortex_panic; +use vortex_error::vortex_bail; use vortex_mask::Mask; use crate::ArrayRef; @@ -155,18 +155,24 @@ pub trait ArrayBuilder: Send { /// The inner part of `extend_from_array`. /// + /// Canonicalizing the array (and computing its validity mask) is fallible, so this method + /// returns a [`VortexResult`]. + /// /// # Safety /// /// The array that must have an equal [`DType`] to the array builder's `DType` (with nullability /// superset semantics). - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()>; /// Extends the array with the provided array, canonicalizing if necessary. /// /// Implementors must validate that the passed in [`ArrayRef`] has the correct [`DType`]. - fn extend_from_array(&mut self, array: &ArrayRef) { + /// + /// Canonicalizing the array (and computing its validity mask) is fallible, so this method + /// returns a [`VortexResult`]. + fn extend_from_array(&mut self, array: &ArrayRef) -> VortexResult<()> { if !self.dtype().eq_with_nullability_superset(array.dtype()) { - vortex_panic!( + vortex_bail!( "tried to extend a builder with `DType` {} with an array with `DType {}", self.dtype(), array.dtype() diff --git a/vortex-array/src/builders/null.rs b/vortex-array/src/builders/null.rs index 45b6e7ed974..b2c032b12d4 100644 --- a/vortex-array/src/builders/null.rs +++ b/vortex-array/src/builders/null.rs @@ -69,8 +69,9 @@ impl ArrayBuilder for NullBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { self.append_nulls(array.len()); + Ok(()) } fn reserve_exact(&mut self, _additional: usize) {} diff --git a/vortex-array/src/builders/primitive.rs b/vortex-array/src/builders/primitive.rs index 657f4b1a603..56e0f5f3f2c 100644 --- a/vortex-array/src/builders/primitive.rs +++ b/vortex-array/src/builders/primitive.rs @@ -5,7 +5,6 @@ use std::any::Any; use std::mem::MaybeUninit; use vortex_buffer::BufferMut; -use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_ensure; use vortex_mask::Mask; @@ -19,8 +18,6 @@ use crate::builders::ArrayBuilder; use crate::builders::DEFAULT_BUILDER_CAPACITY; use crate::builders::LazyBitBufferBuilder; use crate::canonical::Canonical; -#[expect(deprecated)] -use crate::canonical::ToCanonical as _; use crate::dtype::DType; use crate::dtype::NativePType; use crate::dtype::Nullability; @@ -177,9 +174,9 @@ impl ArrayBuilder for PrimitiveBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let array = array.to_primitive(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let array = array.clone().execute::(&mut ctx)?; // This should be checked in `extend_from_array` but we can check it again. debug_assert_eq!( @@ -192,14 +189,11 @@ impl ArrayBuilder for PrimitiveBuilder { self.nulls.append_validity_mask( &array .as_ref() - .validity() - .vortex_expect("validity_mask") - .execute_mask( - array.as_ref().len(), - &mut LEGACY_SESSION.create_execution_ctx(), - ) - .vortex_expect("Failed to compute validity mask"), + .validity()? + .execute_mask(array.as_ref().len(), &mut ctx)?, ); + + Ok(()) } fn reserve_exact(&mut self, additional: usize) { diff --git a/vortex-array/src/builders/struct_.rs b/vortex-array/src/builders/struct_.rs index d396cf186a0..480c5167405 100644 --- a/vortex-array/src/builders/struct_.rs +++ b/vortex-array/src/builders/struct_.rs @@ -22,8 +22,6 @@ use crate::builders::DEFAULT_BUILDER_CAPACITY; use crate::builders::LazyBitBufferBuilder; use crate::builders::builder_with_capacity; use crate::canonical::Canonical; -#[expect(deprecated)] -use crate::canonical::ToCanonical as _; use crate::dtype::DType; use crate::dtype::Nullability; use crate::dtype::StructFields; @@ -168,24 +166,21 @@ impl ArrayBuilder for StructBuilder { self.append_value(scalar.as_struct()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let array = array.to_struct(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let array = array.clone().execute::(&mut ctx)?; for (a, builder) in array .iter_unmasked_fields() .zip_eq(self.builders.iter_mut()) { - builder.extend_from_array(a); + builder.extend_from_array(a)?; } - self.nulls.append_validity_mask( - &array - .validity() - .vortex_expect("validity_mask") - .execute_mask(array.len(), &mut LEGACY_SESSION.create_execution_ctx()) - .vortex_expect("Failed to compute validity mask"), - ); + self.nulls + .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); + + Ok(()) } fn reserve_exact(&mut self, capacity: usize) { diff --git a/vortex-array/src/builders/varbinview.rs b/vortex-array/src/builders/varbinview.rs index 392bef59997..8a41480bc11 100644 --- a/vortex-array/src/builders/varbinview.rs +++ b/vortex-array/src/builders/varbinview.rs @@ -28,8 +28,6 @@ use crate::arrays::varbinview::compact::BufferUtilization; use crate::builders::ArrayBuilder; use crate::builders::LazyBitBufferBuilder; use crate::canonical::Canonical; -#[expect(deprecated)] -use crate::canonical::ToCanonical as _; use crate::dtype::DType; use crate::scalar::Scalar; @@ -294,16 +292,12 @@ impl ArrayBuilder for VarBinViewBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) { - #[expect(deprecated)] - let array = array.to_varbinview(); + unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + let array = array.clone().execute::(&mut ctx)?; self.flush_in_progress(); - let mask = array - .validity() - .vortex_expect("validity_mask") - .execute_mask(array.len(), &mut LEGACY_SESSION.create_execution_ctx()) - .vortex_expect("Failed to compute validity mask"); + let mask = array.validity()?.execute_mask(array.len(), &mut ctx)?; self.push_only_validity_mask(&mask); @@ -346,6 +340,8 @@ impl ArrayBuilder for VarBinViewBuilder { } }, } + + Ok(()) } fn reserve_exact(&mut self, additional: usize) { @@ -890,7 +886,7 @@ mod tests { let mut builder = VarBinViewBuilder::with_capacity(DType::Utf8(Nullability::Nullable), 10); builder.append_value("Hello1"); - builder.extend_from_array(&array); + builder.extend_from_array(&array).unwrap(); builder.append_nulls(2); builder.append_value("Hello3"); From c27328987c13675379084d21d16d76e3fc60f783 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 13:39:24 +0000 Subject: [PATCH 2/4] Thread `ExecutionCtx` through builder extend methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ArrayBuilder::extend_from_array` and `extend_from_array_unchecked` canonicalize their input (and compute validity masks), which requires an `ExecutionCtx`. Previously each implementation minted a fresh `LEGACY_SESSION` context internally on every call — once per chunk for chunked input — as called out by the `ListViewBuilder` TODO. Add a `&mut ExecutionCtx` parameter to both methods so the caller's session is reused. The internal builder-to-builder calls (struct fields, list/fsl elements, listview sizes) now thread the same context through instead of spawning new ones. `ListBuilder`/`ListViewBuilder`/`FixedSizeListBuilder`'s public `append_array_as_list` gains the same parameter, and the sparse canonicalization fill helpers thread the context through their fill loops. Call sites that already hold a context (the `append_to_builder` VTable paths, list view rebuild) pass it straight through. The few entry points without one — `VarBinViewArray::compact_with_threshold` (called from FFI and fuzz targets) and tests/benches — mint a `LEGACY_SESSION` context locally. Signed-off-by: Joseph Isaacs --- encodings/experimental/onpair/src/array.rs | 1 + .../src/bitpacking/array/bitpack_compress.rs | 2 +- encodings/fsst/src/array.rs | 1 + encodings/sparse/src/canonical.rs | 21 +++-- .../benches/listview_builder_extend.rs | 6 +- .../fns/uncompressed_size_in_bytes/mod.rs | 5 +- vortex-array/src/array/vtable/mod.rs | 2 +- .../src/arrays/constant/vtable/mod.rs | 2 +- vortex-array/src/arrays/listview/rebuild.rs | 2 +- vortex-array/src/arrays/patched/vtable/mod.rs | 2 +- vortex-array/src/arrays/varbinview/compact.rs | 5 +- vortex-array/src/builders/bool.rs | 14 ++-- vortex-array/src/builders/decimal.rs | 18 +++-- vortex-array/src/builders/extension.rs | 15 ++-- vortex-array/src/builders/fixed_size_list.rs | 81 ++++++++++++++----- vortex-array/src/builders/list.rs | 58 ++++++++----- vortex-array/src/builders/listview.rs | 77 +++++++++++------- vortex-array/src/builders/mod.rs | 19 +++-- vortex-array/src/builders/null.rs | 7 +- vortex-array/src/builders/primitive.rs | 16 ++-- vortex-array/src/builders/struct_.rs | 16 ++-- vortex-array/src/builders/varbinview.rs | 18 +++-- 22 files changed, 261 insertions(+), 127 deletions(-) diff --git a/encodings/experimental/onpair/src/array.rs b/encodings/experimental/onpair/src/array.rs index e576af92c11..106eb409de1 100644 --- a/encodings/experimental/onpair/src/array.rs +++ b/encodings/experimental/onpair/src/array.rs @@ -482,6 +482,7 @@ impl VTable for OnPair { .clone() .execute::(ctx)? .into_array(), + ctx, )?; return Ok(()); }; diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs index f8c290cee75..c77f75cf0ba 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs @@ -519,7 +519,7 @@ mod test { let mut primitive_builder = PrimitiveBuilder::::with_capacity(chunked.dtype().nullability(), 10 * 100); - primitive_builder.extend_from_array(&chunked)?; + primitive_builder.extend_from_array(&chunked, &mut ctx)?; let ca_into = primitive_builder.finish(); assert_arrays_eq!(into_ca, ca_into); diff --git a/encodings/fsst/src/array.rs b/encodings/fsst/src/array.rs index 8315cda0c9c..218e8b78413 100644 --- a/encodings/fsst/src/array.rs +++ b/encodings/fsst/src/array.rs @@ -292,6 +292,7 @@ impl VTable for FSST { .clone() .execute::(ctx)? .into_array(), + ctx, )?; return Ok(()); }; diff --git a/encodings/sparse/src/canonical.rs b/encodings/sparse/src/canonical.rs index dd84123851a..17b4241f293 100644 --- a/encodings/sparse/src/canonical.rs +++ b/encodings/sparse/src/canonical.rs @@ -239,6 +239,7 @@ fn execute_sparse_lists_inner( &mut builder, fill_elements.as_ref(), sparse_idx - next_index, + ctx, ); if patch_valid { @@ -246,7 +247,7 @@ fn execute_sparse_lists_inner( .list_elements_at(patch_idx) .vortex_expect("list_elements_at"); builder - .append_array_as_list(&patch_list) + .append_array_as_list(&patch_list, ctx) .vortex_expect("Failed to append sparse value"); } else { builder.append_null(); @@ -255,7 +256,7 @@ fn execute_sparse_lists_inner( next_index = sparse_idx + 1; } - append_list_fill(&mut builder, fill_elements.as_ref(), len - next_index); + append_list_fill(&mut builder, fill_elements.as_ref(), len - next_index, ctx); builder.finish() } @@ -328,6 +329,7 @@ fn execute_sparse_fixed_size_list_inner( &mut builder, fill_elements.as_ref(), sparse_idx - next_index, + ctx, ); // Append the patch value, handling null patches by appending defaults. @@ -336,7 +338,7 @@ fn execute_sparse_fixed_size_list_inner( .fixed_size_list_elements_at(patch_idx) .vortex_expect("fixed_size_list_elements_at"); builder - .append_array_as_list(&patch_list) + .append_array_as_list(&patch_list, ctx) .vortex_expect("Failed to append sparse fixed-size-list value"); } else { builder.append_null(); @@ -346,7 +348,12 @@ fn execute_sparse_fixed_size_list_inner( } // Fill remaining positions after last patch. - append_fixed_size_list_fill(&mut builder, fill_elements.as_ref(), array_len - next_index); + append_fixed_size_list_fill( + &mut builder, + fill_elements.as_ref(), + array_len - next_index, + ctx, + ); builder.finish_into_fixed_size_list() } @@ -367,11 +374,12 @@ fn append_list_fill( builder: &mut ListViewBuilder, fill_elements: Option<&ArrayRef>, count: usize, + ctx: &mut ExecutionCtx, ) { if let Some(fill_elements) = fill_elements { for _ in 0..count { builder - .append_array_as_list(fill_elements) + .append_array_as_list(fill_elements, ctx) .vortex_expect("Failed to append sparse fill value"); } } else { @@ -383,11 +391,12 @@ fn append_fixed_size_list_fill( builder: &mut FixedSizeListBuilder, fill_elements: Option<&ArrayRef>, count: usize, + ctx: &mut ExecutionCtx, ) { if let Some(fill_elements) = fill_elements { for _ in 0..count { builder - .append_array_as_list(fill_elements) + .append_array_as_list(fill_elements, ctx) .vortex_expect("Failed to append sparse fixed-size-list fill value"); } } else { diff --git a/vortex-array/benches/listview_builder_extend.rs b/vortex-array/benches/listview_builder_extend.rs index 8f686e6616b..5aa80fa3faf 100644 --- a/vortex-array/benches/listview_builder_extend.rs +++ b/vortex-array/benches/listview_builder_extend.rs @@ -8,6 +8,8 @@ use std::sync::Arc; use divan::Bencher; use vortex_array::IntoArray; +use vortex_array::LEGACY_SESSION; +use vortex_array::VortexSessionExecute; use vortex_array::arrays::ListViewArray; use vortex_array::arrays::PrimitiveArray; use vortex_array::builders::ArrayBuilder; @@ -71,7 +73,7 @@ fn extend_from_array_zctl(bencher: Bencher, (num_lists, list_size): (usize, usiz num_lists, ); builder - .extend_from_array(source) + .extend_from_array(source, &mut LEGACY_SESSION.create_execution_ctx()) .vortex_expect("extend_from_array"); divan::black_box(builder.finish_into_listview()) }); @@ -95,7 +97,7 @@ fn extend_from_array_non_zctl_overlapping( num_lists, ); builder - .extend_from_array(source) + .extend_from_array(source, &mut LEGACY_SESSION.create_execution_ctx()) .vortex_expect("extend_from_array"); divan::black_box(builder.finish_into_listview()) }); diff --git a/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs b/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs index d83a060edc0..a4a2737c991 100644 --- a/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs @@ -344,8 +344,11 @@ mod tests { fn materialized_uncompressed_size_in_bytes(array: &ArrayRef) -> u64 { let mut builder = builder_with_capacity(array.dtype(), array.len()); + let mut ctx = LEGACY_SESSION.create_execution_ctx(); unsafe { - builder.extend_from_array_unchecked(array).unwrap(); + builder + .extend_from_array_unchecked(array, &mut ctx) + .unwrap(); } builder.finish().nbytes() } diff --git a/vortex-array/src/array/vtable/mod.rs b/vortex-array/src/array/vtable/mod.rs index 54941940da1..c23d8ed09a9 100644 --- a/vortex-array/src/array/vtable/mod.rs +++ b/vortex-array/src/array/vtable/mod.rs @@ -151,7 +151,7 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug { .clone() .execute::(ctx)? .into_array(); - builder.extend_from_array(&canonical)?; + builder.extend_from_array(&canonical, ctx)?; Ok(()) } diff --git a/vortex-array/src/arrays/constant/vtable/mod.rs b/vortex-array/src/arrays/constant/vtable/mod.rs index fd32de8de76..2b7b54d6713 100644 --- a/vortex-array/src/arrays/constant/vtable/mod.rs +++ b/vortex-array/src/arrays/constant/vtable/mod.rs @@ -236,7 +236,7 @@ impl VTable for Constant { .clone() .execute::(ctx)? .into_array(); - builder.extend_from_array(&canonical)?; + builder.extend_from_array(&canonical, ctx)?; } } diff --git a/vortex-array/src/arrays/listview/rebuild.rs b/vortex-array/src/arrays/listview/rebuild.rs index 1fe17e01cc2..3487e083835 100644 --- a/vortex-array/src/arrays/listview/rebuild.rs +++ b/vortex-array/src/arrays/listview/rebuild.rs @@ -309,7 +309,7 @@ impl ListViewArray { new_offsets.push(n_elements); new_sizes.push(size); - new_elements_builder.extend_from_array(&elements_canonical.slice(start..stop)?)?; + new_elements_builder.extend_from_array(&elements_canonical.slice(start..stop)?, ctx)?; n_elements += num_traits::cast(size).vortex_expect("Cast failed"); } diff --git a/vortex-array/src/arrays/patched/vtable/mod.rs b/vortex-array/src/arrays/patched/vtable/mod.rs index 1bdd2a37a60..c44332b784b 100644 --- a/vortex-array/src/arrays/patched/vtable/mod.rs +++ b/vortex-array/src/arrays/patched/vtable/mod.rs @@ -198,7 +198,7 @@ impl VTable for Patched { .clone() .execute::(ctx)? .into_array(); - builder.extend_from_array(&canonical)?; + builder.extend_from_array(&canonical, ctx)?; return Ok(()); } diff --git a/vortex-array/src/arrays/varbinview/compact.rs b/vortex-array/src/arrays/varbinview/compact.rs index 3c96d8886a2..a159f9781d0 100644 --- a/vortex-array/src/arrays/varbinview/compact.rs +++ b/vortex-array/src/arrays/varbinview/compact.rs @@ -136,7 +136,10 @@ impl VarBinViewArray { self.len(), buffer_utilization_threshold, ); - builder.extend_from_array(&self.clone().into_array())?; + // This array is already canonical, so executing it through the builder is cheap; we only + // need a context to satisfy the `extend_from_array` signature. + let mut ctx = LEGACY_SESSION.create_execution_ctx(); + builder.extend_from_array(&self.clone().into_array(), &mut ctx)?; Ok(builder.finish_into_varbinview()) } } diff --git a/vortex-array/src/builders/bool.rs b/vortex-array/src/builders/bool.rs index a5a56038c59..8067c651637 100644 --- a/vortex-array/src/builders/bool.rs +++ b/vortex-array/src/builders/bool.rs @@ -10,9 +10,8 @@ use vortex_error::vortex_ensure; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::BoolArray; use crate::arrays::bool::BoolArrayExt; use crate::builders::ArrayBuilder; @@ -112,16 +111,19 @@ impl ArrayBuilder for BoolBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let bool_array = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let bool_array = array.clone().execute::(ctx)?; self.inner.append_buffer(&bool_array.to_bit_buffer()); self.nulls.append_validity_mask( &bool_array .as_ref() .validity()? - .execute_mask(bool_array.as_ref().len(), &mut ctx)?, + .execute_mask(bool_array.as_ref().len(), ctx)?, ); Ok(()) diff --git a/vortex-array/src/builders/decimal.rs b/vortex-array/src/builders/decimal.rs index 351a7e078d2..be04acd627f 100644 --- a/vortex-array/src/builders/decimal.rs +++ b/vortex-array/src/builders/decimal.rs @@ -12,9 +12,8 @@ use vortex_error::vortex_panic; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::DecimalArray; use crate::builders::ArrayBuilder; use crate::builders::DEFAULT_BUILDER_CAPACITY; @@ -193,9 +192,12 @@ impl ArrayBuilder for DecimalBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let decimal_array = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let decimal_array = array.clone().execute::(ctx)?; match_each_decimal_value_type!(decimal_array.values_type(), |D| { // Extends the values buffer from another buffer of type D where D can be coerced to the @@ -208,7 +210,7 @@ impl ArrayBuilder for DecimalBuilder { &decimal_array .as_ref() .validity()? - .execute_mask(decimal_array.as_ref().len(), &mut ctx)?, + .execute_mask(decimal_array.as_ref().len(), ctx)?, ); Ok(()) @@ -323,7 +325,9 @@ mod tests { let i8s = i8s.finish(); let mut i128s = DecimalBuilder::new::(DecimalDType::new(2, 1), false.into()); - i128s.extend_from_array(&i8s).unwrap(); + i128s + .extend_from_array(&i8s, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); let i128s = i128s.finish(); for i in 0..i8s.len() { diff --git a/vortex-array/src/builders/extension.rs b/vortex-array/src/builders/extension.rs index d079a6375df..aae352c0740 100644 --- a/vortex-array/src/builders/extension.rs +++ b/vortex-array/src/builders/extension.rs @@ -8,9 +8,8 @@ use vortex_error::vortex_ensure; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::ExtensionArray; use crate::arrays::extension::ExtensionArrayExt; use crate::builders::ArrayBuilder; @@ -99,10 +98,14 @@ impl ArrayBuilder for ExtensionBuilder { self.append_value(scalar.as_extension()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let ext_array = array.clone().execute::(&mut ctx)?; - self.storage.extend_from_array(ext_array.storage_array()) + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let ext_array = array.clone().execute::(ctx)?; + self.storage + .extend_from_array(ext_array.storage_array(), ctx) } fn reserve_exact(&mut self, capacity: usize) { diff --git a/vortex-array/src/builders/fixed_size_list.rs b/vortex-array/src/builders/fixed_size_list.rs index 174c1c80178..f362645d2da 100644 --- a/vortex-array/src/builders/fixed_size_list.rs +++ b/vortex-array/src/builders/fixed_size_list.rs @@ -12,9 +12,8 @@ use vortex_error::vortex_panic; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::FixedSizeListArray; use crate::arrays::fixed_size_list::FixedSizeListArrayExt; use crate::builders::ArrayBuilder; @@ -81,7 +80,11 @@ impl FixedSizeListBuilder { /// /// Note that the list entry will be non-null but the elements themselves are allowed to be null /// (only if the elements [`DType`] is nullable, of course). - pub fn append_array_as_list(&mut self, array: &ArrayRef) -> VortexResult<()> { + pub fn append_array_as_list( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { vortex_ensure!( array.dtype() == self.element_dtype(), "Array dtype {:?} does not match list element dtype {:?}", @@ -95,7 +98,7 @@ impl FixedSizeListBuilder { self.list_size() ); - self.elements_builder.extend_from_array(array)?; + self.elements_builder.extend_from_array(array, ctx)?; self.nulls.append_non_null(); Ok(()) @@ -235,16 +238,20 @@ impl ArrayBuilder for FixedSizeListBuilder { /// This will increase the capacity if extending with this `array` would go past the original /// capacity. - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let fsl = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let fsl = array.clone().execute::(ctx)?; if fsl.is_empty() { return Ok(()); } - self.elements_builder.extend_from_array(fsl.elements())?; + self.elements_builder + .extend_from_array(fsl.elements(), ctx)?; self.nulls - .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); + .append_validity_mask(&array.validity()?.execute_mask(array.len(), ctx)?); Ok(()) } @@ -670,8 +677,12 @@ mod tests { let mut builder = FixedSizeListBuilder::with_capacity(dtype, 2, Nullable, 0); let source_array = source.into_array(); - builder.extend_from_array(&source_array).unwrap(); - builder.extend_from_array(&source_array).unwrap(); + builder + .extend_from_array(&source_array, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); + builder + .extend_from_array(&source_array, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 6); @@ -746,8 +757,18 @@ mod tests { let mut builder = FixedSizeListBuilder::with_capacity(dtype, 0, Nullable, 0); - builder.extend_from_array(&source1.into_array()).unwrap(); - builder.extend_from_array(&source2.into_array()).unwrap(); + builder + .extend_from_array( + &source1.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); + builder + .extend_from_array( + &source2.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 5); @@ -823,7 +844,12 @@ mod tests { .unwrap(); // Extend with empty array (should be no-op). - builder.extend_from_array(&source.into_array()).unwrap(); + builder + .extend_from_array( + &source.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 1); @@ -860,7 +886,12 @@ mod tests { Validity::AllValid, 1, ); - builder.extend_from_array(&source.into_array()).unwrap(); + builder + .extend_from_array( + &source.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); let fsl = builder.finish(); assert_eq!(fsl.len(), 6); @@ -994,7 +1025,9 @@ mod tests { // Append a primitive array as a single list entry. let arr1 = buffer![1i32, 2, 3].into_array(); - builder.append_array_as_list(&arr1).unwrap(); + builder + .append_array_as_list(&arr1, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Interleave with a list scalar. builder @@ -1010,7 +1043,9 @@ mod tests { // Append another primitive array as a single list entry. let arr2 = buffer![4i32, 5, 6].into_array(); - builder.append_array_as_list(&arr2).unwrap(); + builder + .append_array_as_list(&arr2, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Interleave with another list scalar. builder @@ -1040,11 +1075,19 @@ mod tests { let mut builder = FixedSizeListBuilder::with_capacity(Arc::clone(&dtype), 3, NonNullable, 10); let wrong_dtype_arr = buffer![1i64, 2, 3].into_array(); - assert!(builder.append_array_as_list(&wrong_dtype_arr).is_err()); + assert!( + builder + .append_array_as_list(&wrong_dtype_arr, &mut LEGACY_SESSION.create_execution_ctx()) + .is_err() + ); // Test length mismatch error. let mut builder = FixedSizeListBuilder::with_capacity(dtype, 3, NonNullable, 10); let wrong_len_arr = buffer![1i32, 2].into_array(); - assert!(builder.append_array_as_list(&wrong_len_arr).is_err()); + assert!( + builder + .append_array_as_list(&wrong_len_arr, &mut LEGACY_SESSION.create_execution_ctx()) + .is_err() + ); } } diff --git a/vortex-array/src/builders/list.rs b/vortex-array/src/builders/list.rs index 69584a81901..f1f824c902e 100644 --- a/vortex-array/src/builders/list.rs +++ b/vortex-array/src/builders/list.rs @@ -13,9 +13,8 @@ use vortex_mask::Mask; use crate::ArrayRef; use crate::Canonical; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::ListArray; use crate::arrays::ListViewArray; use crate::arrays::PrimitiveArray; @@ -97,7 +96,11 @@ impl ListBuilder { /// /// Note that the list entry will be non-null but the elements themselves are allowed to be null /// (only if the elements [`DType`] in nullable, of course). - pub fn append_array_as_list(&mut self, array: &ArrayRef) -> VortexResult<()> { + pub fn append_array_as_list( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { vortex_ensure!( array.dtype() == self.element_dtype(), "Array dtype {:?} does not match list element dtype {:?}", @@ -105,7 +108,7 @@ impl ListBuilder { self.element_dtype() ); - self.elements_builder.extend_from_array(array)?; + self.elements_builder.extend_from_array(array, ctx)?; self.nulls.append_non_null(); self.offsets_builder.append_value( O::from_usize(self.elements_builder.len()) @@ -219,27 +222,31 @@ impl ArrayBuilder for ListBuilder { self.append_value(scalar.as_list()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let list = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let list = array.clone().execute::(ctx)?; if list.is_empty() { return Ok(()); } // Append validity information. self.nulls - .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); + .append_validity_mask(&array.validity()?.execute_mask(array.len(), ctx)?); // Note that `ListViewArray` has `n` offsets and sizes, not `n+1` offsets like `ListArray`. let elements = list.elements(); - let offsets = list.offsets().clone().execute::(&mut ctx)?; - let sizes = list.sizes().clone().execute::(&mut ctx)?; + let offsets = list.offsets().clone().execute::(ctx)?; + let sizes = list.sizes().clone().execute::(ctx)?; fn extend_inner( builder: &mut ListBuilder, new_elements: &ArrayRef, new_offsets: &[OffsetType], new_sizes: &[SizeType], + ctx: &mut ExecutionCtx, ) -> VortexResult<()> where O: IntegerPType, @@ -260,7 +267,9 @@ impl ArrayBuilder for ListBuilder { if size > 0 { let list_elements = new_elements.slice(offset..offset + size)?; - builder.elements_builder.extend_from_array(&list_elements)?; + builder + .elements_builder + .extend_from_array(&list_elements, ctx)?; curr_offset += size; } @@ -284,6 +293,7 @@ impl ArrayBuilder for ListBuilder { elements, offsets.as_slice::(), sizes.as_slice::(), + ctx, ) }) }) @@ -450,13 +460,13 @@ mod tests { let mut ctx = LEGACY_SESSION.create_execution_ctx(); let mut builder = ListBuilder::::with_capacity(Arc::new(I32.into()), Nullable, 18, 9); - builder.extend_from_array(&list).unwrap(); - builder.extend_from_array(&list).unwrap(); + builder.extend_from_array(&list, &mut ctx).unwrap(); + builder.extend_from_array(&list, &mut ctx).unwrap(); builder - .extend_from_array(&list.slice(0..0).unwrap()) + .extend_from_array(&list.slice(0..0).unwrap(), &mut ctx) .unwrap(); builder - .extend_from_array(&list.slice(1..3).unwrap()) + .extend_from_array(&list.slice(1..3).unwrap(), &mut ctx) .unwrap(); #[expect(deprecated)] @@ -645,7 +655,9 @@ mod tests { // Append a primitive array as a single list entry. let arr1 = buffer![1i32, 2, 3].into_array(); - builder.append_array_as_list(&arr1).unwrap(); + builder + .append_array_as_list(&arr1, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Interleave with a list scalar. builder @@ -661,11 +673,15 @@ mod tests { // Append another primitive array as a single list entry. let arr2 = buffer![4i32, 5].into_array(); - builder.append_array_as_list(&arr2).unwrap(); + builder + .append_array_as_list(&arr2, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Append an empty array as a single list entry (empty list). let arr3 = buffer![0i32; 0].into_array(); - builder.append_array_as_list(&arr3).unwrap(); + builder + .append_array_as_list(&arr3, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Interleave with another list scalar (empty list). builder @@ -690,6 +706,10 @@ mod tests { // Test dtype mismatch error. let mut builder = ListBuilder::::with_capacity(dtype, NonNullable, 20, 10); let wrong_dtype_arr = buffer![1i64, 2, 3].into_array(); - assert!(builder.append_array_as_list(&wrong_dtype_arr).is_err()); + assert!( + builder + .append_array_as_list(&wrong_dtype_arr, &mut LEGACY_SESSION.create_execution_ctx()) + .is_err() + ); } } diff --git a/vortex-array/src/builders/listview.rs b/vortex-array/src/builders/listview.rs index 84109cf8dbb..a33fd1551d1 100644 --- a/vortex-array/src/builders/listview.rs +++ b/vortex-array/src/builders/listview.rs @@ -20,8 +20,7 @@ use vortex_mask::Mask; use crate::ArrayRef; use crate::Canonical; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; +use crate::ExecutionCtx; use crate::array::IntoArray; use crate::arrays::ListViewArray; use crate::arrays::PrimitiveArray; @@ -117,7 +116,11 @@ impl ListViewBuilder { /// /// Note that the list entry will be non-null but the elements themselves are allowed to be null /// (only if the elements [`DType`] is nullable, of course). - pub fn append_array_as_list(&mut self, array: &ArrayRef) -> VortexResult<()> { + pub fn append_array_as_list( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { vortex_ensure!( array.dtype() == self.element_dtype(), "Array dtype {:?} does not match list element dtype {:?}", @@ -135,7 +138,7 @@ impl ListViewBuilder { "appending this list would cause an offset overflow" ); - self.elements_builder.extend_from_array(array)?; + self.elements_builder.extend_from_array(array, ctx)?; self.nulls.append_non_null(); self.offsets_builder.append_value( @@ -291,32 +294,30 @@ impl ArrayBuilder for ListViewBuilder { self.append_value(list_scalar) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - // TODO: The `ArrayBuilder` trait does not thread an `ExecutionCtx` through its extend - // methods, so we are forced to mint a fresh `LEGACY_SESSION` context here on every call - // (which for chunked input means once per chunk). Once the trait carries a `&mut - // ExecutionCtx`, the caller's session should be reused instead. - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - - let listview = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let listview = array.clone().execute::(ctx)?; if listview.is_empty() { return Ok(()); } // Normalize to an exact zero-copy-to-list layout and then bulk append. This avoids the // very expensive scalar_at-per-list path for overlapping / out-of-order list views. - let listview = listview.rebuild(ListViewRebuildMode::MakeExact, &mut ctx)?; + let listview = listview.rebuild(ListViewRebuildMode::MakeExact, ctx)?; debug_assert!(listview.is_zero_copy_to_list()); self.nulls - .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); + .append_validity_mask(&array.validity()?.execute_mask(array.len(), ctx)?); // Bulk append the new elements (which should have no gaps or overlaps). let old_elements_len = self.elements_builder.len(); self.elements_builder .reserve_exact(listview.elements().len()); self.elements_builder - .extend_from_array(listview.elements())?; + .extend_from_array(listview.elements(), ctx)?; let new_elements_len = self.elements_builder.len(); // Reserve enough space for the new views. @@ -329,7 +330,7 @@ impl ArrayBuilder for ListViewBuilder { .sizes() .clone() .cast(self.sizes_builder.dtype().clone())?; - self.sizes_builder.extend_from_array(&cast_sizes)?; + self.sizes_builder.extend_from_array(&cast_sizes, ctx)?; // Now we need to adjust all of the offsets by adding the current number of elements in the // builder. @@ -337,10 +338,7 @@ impl ArrayBuilder for ListViewBuilder { let uninit_range = self.offsets_builder.uninit_range(extend_length); // This should be cheap because we didn't compress after rebuilding. - let new_offsets = listview - .offsets() - .clone() - .execute::(&mut ctx)?; + let new_offsets = listview.offsets().clone().execute::(ctx)?; match_each_integer_ptype!(new_offsets.ptype(), |A| { adjust_and_extend_offsets::( @@ -423,6 +421,8 @@ mod tests { use super::ListViewBuilder; use crate::IntoArray; + use crate::LEGACY_SESSION; + use crate::VortexSessionExecute; use crate::arrays::ListArray; use crate::arrays::ListViewArray; use crate::arrays::listview::ListViewArrayExt; @@ -646,7 +646,12 @@ mod tests { .unwrap(); // Extend from the ListArray. - builder.extend_from_array(&source.into_array()).unwrap(); + builder + .extend_from_array( + &source.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); // Extend from empty array (should be no-op). let empty_source = ListArray::from_iter_opt_slow::>( @@ -655,7 +660,10 @@ mod tests { ) .unwrap(); builder - .extend_from_array(&empty_source.into_array()) + .extend_from_array( + &empty_source.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) .unwrap(); let listview = builder.finish_into_listview(); @@ -710,7 +718,12 @@ mod tests { let mut builder = ListViewBuilder::::with_capacity(Arc::clone(&dtype), Nullable, 0, 0); - builder.extend_from_array(&source.into_array()).unwrap(); + builder + .extend_from_array( + &source.into_array(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); let listview = builder.finish_into_listview(); assert_eq!(listview.len(), 3); @@ -763,7 +776,9 @@ mod tests { // Append a primitive array as a single list entry. let arr1 = buffer![1i32, 2, 3].into_array(); - builder.append_array_as_list(&arr1).unwrap(); + builder + .append_array_as_list(&arr1, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Interleave with a list scalar. builder @@ -779,11 +794,15 @@ mod tests { // Append another primitive array as a single list entry. let arr2 = buffer![4i32, 5].into_array(); - builder.append_array_as_list(&arr2).unwrap(); + builder + .append_array_as_list(&arr2, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Append an empty array as a single list entry (empty list). let arr3 = buffer![0i32; 0].into_array(); - builder.append_array_as_list(&arr3).unwrap(); + builder + .append_array_as_list(&arr3, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); // Interleave with another list scalar. builder @@ -814,6 +833,10 @@ mod tests { // Test dtype mismatch error. let mut builder = ListViewBuilder::::with_capacity(dtype, NonNullable, 20, 10); let wrong_dtype_arr = buffer![1i64, 2, 3].into_array(); - assert!(builder.append_array_as_list(&wrong_dtype_arr).is_err()); + assert!( + builder + .append_array_as_list(&wrong_dtype_arr, &mut LEGACY_SESSION.create_execution_ctx()) + .is_err() + ); } } diff --git a/vortex-array/src/builders/mod.rs b/vortex-array/src/builders/mod.rs index 39e39d94b8d..15b508a403b 100644 --- a/vortex-array/src/builders/mod.rs +++ b/vortex-array/src/builders/mod.rs @@ -38,6 +38,7 @@ use vortex_error::vortex_bail; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::canonical::Canonical; use crate::dtype::DType; use crate::match_each_decimal_value_type; @@ -155,22 +156,26 @@ pub trait ArrayBuilder: Send { /// The inner part of `extend_from_array`. /// - /// Canonicalizing the array (and computing its validity mask) is fallible, so this method - /// returns a [`VortexResult`]. + /// Canonicalizing the array (and computing its validity mask) is fallible and is performed + /// using the provided [`ExecutionCtx`], so this method returns a [`VortexResult`]. /// /// # Safety /// /// The array that must have an equal [`DType`] to the array builder's `DType` (with nullability /// superset semantics). - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()>; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()>; /// Extends the array with the provided array, canonicalizing if necessary. /// /// Implementors must validate that the passed in [`ArrayRef`] has the correct [`DType`]. /// - /// Canonicalizing the array (and computing its validity mask) is fallible, so this method - /// returns a [`VortexResult`]. - fn extend_from_array(&mut self, array: &ArrayRef) -> VortexResult<()> { + /// Canonicalizing the array (and computing its validity mask) is fallible and is performed + /// using the provided [`ExecutionCtx`], so this method returns a [`VortexResult`]. + fn extend_from_array(&mut self, array: &ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult<()> { if !self.dtype().eq_with_nullability_superset(array.dtype()) { vortex_bail!( "tried to extend a builder with `DType` {} with an array with `DType {}", @@ -180,7 +185,7 @@ pub trait ArrayBuilder: Send { } // SAFETY: We checked that the array had a valid `DType` above. - unsafe { self.extend_from_array_unchecked(array) } + unsafe { self.extend_from_array_unchecked(array, ctx) } } /// Allocate space for extra `additional` items diff --git a/vortex-array/src/builders/null.rs b/vortex-array/src/builders/null.rs index b2c032b12d4..79bcf49722d 100644 --- a/vortex-array/src/builders/null.rs +++ b/vortex-array/src/builders/null.rs @@ -8,6 +8,7 @@ use vortex_error::vortex_ensure; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; use crate::arrays::NullArray; use crate::builders::ArrayBuilder; @@ -69,7 +70,11 @@ impl ArrayBuilder for NullBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + _ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { self.append_nulls(array.len()); Ok(()) } diff --git a/vortex-array/src/builders/primitive.rs b/vortex-array/src/builders/primitive.rs index 56e0f5f3f2c..64c97338d6e 100644 --- a/vortex-array/src/builders/primitive.rs +++ b/vortex-array/src/builders/primitive.rs @@ -10,9 +10,8 @@ use vortex_error::vortex_ensure; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::PrimitiveArray; use crate::builders::ArrayBuilder; use crate::builders::DEFAULT_BUILDER_CAPACITY; @@ -174,9 +173,12 @@ impl ArrayBuilder for PrimitiveBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let array = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let array = array.clone().execute::(ctx)?; // This should be checked in `extend_from_array` but we can check it again. debug_assert_eq!( @@ -190,7 +192,7 @@ impl ArrayBuilder for PrimitiveBuilder { &array .as_ref() .validity()? - .execute_mask(array.as_ref().len(), &mut ctx)?, + .execute_mask(array.as_ref().len(), ctx)?, ); Ok(()) @@ -364,6 +366,8 @@ mod tests { use vortex_error::VortexExpect; use super::*; + use crate::LEGACY_SESSION; + use crate::VortexSessionExecute; use crate::assert_arrays_eq; /// REGRESSION TEST: This test verifies that multiple sequential ranges have correct offsets. diff --git a/vortex-array/src/builders/struct_.rs b/vortex-array/src/builders/struct_.rs index 480c5167405..aa8fc99b32e 100644 --- a/vortex-array/src/builders/struct_.rs +++ b/vortex-array/src/builders/struct_.rs @@ -12,9 +12,8 @@ use vortex_error::vortex_panic; use vortex_mask::Mask; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::StructArray; use crate::arrays::struct_::StructArrayExt; use crate::builders::ArrayBuilder; @@ -166,19 +165,22 @@ impl ArrayBuilder for StructBuilder { self.append_value(scalar.as_struct()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let array = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let array = array.clone().execute::(ctx)?; for (a, builder) in array .iter_unmasked_fields() .zip_eq(self.builders.iter_mut()) { - builder.extend_from_array(a)?; + builder.extend_from_array(a, ctx)?; } self.nulls - .append_validity_mask(&array.validity()?.execute_mask(array.len(), &mut ctx)?); + .append_validity_mask(&array.validity()?.execute_mask(array.len(), ctx)?); Ok(()) } diff --git a/vortex-array/src/builders/varbinview.rs b/vortex-array/src/builders/varbinview.rs index 8a41480bc11..e2389914582 100644 --- a/vortex-array/src/builders/varbinview.rs +++ b/vortex-array/src/builders/varbinview.rs @@ -19,9 +19,8 @@ use vortex_utils::aliases::hash_map::Entry; use vortex_utils::aliases::hash_map::HashMap; use crate::ArrayRef; +use crate::ExecutionCtx; use crate::IntoArray; -use crate::LEGACY_SESSION; -use crate::VortexSessionExecute; use crate::arrays::VarBinViewArray; use crate::arrays::varbinview::build_views::BinaryView; use crate::arrays::varbinview::compact::BufferUtilization; @@ -292,12 +291,15 @@ impl ArrayBuilder for VarBinViewBuilder { Ok(()) } - unsafe fn extend_from_array_unchecked(&mut self, array: &ArrayRef) -> VortexResult<()> { - let mut ctx = LEGACY_SESSION.create_execution_ctx(); - let array = array.clone().execute::(&mut ctx)?; + unsafe fn extend_from_array_unchecked( + &mut self, + array: &ArrayRef, + ctx: &mut ExecutionCtx, + ) -> VortexResult<()> { + let array = array.clone().execute::(ctx)?; self.flush_in_progress(); - let mask = array.validity()?.execute_mask(array.len(), &mut ctx)?; + let mask = array.validity()?.execute_mask(array.len(), ctx)?; self.push_only_validity_mask(&mask); @@ -886,7 +888,9 @@ mod tests { let mut builder = VarBinViewBuilder::with_capacity(DType::Utf8(Nullability::Nullable), 10); builder.append_value("Hello1"); - builder.extend_from_array(&array).unwrap(); + builder + .extend_from_array(&array, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); builder.append_nulls(2); builder.append_value("Hello3"); From c95497357e83f55ea4a59819bc53386d06edd7a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 14:01:38 +0000 Subject: [PATCH 3/4] Fix execution context threading after develop merge `develop` refactored `rebuild_list_by_list` to use a local `LEGACY_SESSION` context for its hoisted `execute_is_valid` validity check. The merge with the `ExecutionCtx`-threading change left `extend_from_array` receiving that owned local context, so pass `&mut ctx` to match the surrounding loop. Signed-off-by: Joseph Isaacs --- vortex-array/src/arrays/listview/rebuild.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vortex-array/src/arrays/listview/rebuild.rs b/vortex-array/src/arrays/listview/rebuild.rs index 329f6ce7810..9b6a41a30da 100644 --- a/vortex-array/src/arrays/listview/rebuild.rs +++ b/vortex-array/src/arrays/listview/rebuild.rs @@ -315,7 +315,8 @@ impl ListViewArray { new_offsets.push(n_elements); new_sizes.push(size); - new_elements_builder.extend_from_array(&elements_canonical.slice(start..stop)?, ctx)?; + new_elements_builder + .extend_from_array(&elements_canonical.slice(start..stop)?, &mut ctx)?; n_elements += num_traits::cast(size).vortex_expect("Cast failed"); } From 9e0315a7247f6fdcea283ee2a6cf4ef3fb245d19 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 14:09:13 +0000 Subject: [PATCH 4/4] Align listview rebuild with develop: use parameter context The latest `develop` removed the local `LEGACY_SESSION` context shadow from `rebuild_list_by_list` and now threads the `&mut ExecutionCtx` parameter directly. Update the `extend_from_array` call to pass the parameter `ctx` instead of `&mut ctx`, matching the surrounding `execute_is_valid` calls. Signed-off-by: Joseph Isaacs --- vortex-array/src/arrays/listview/rebuild.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vortex-array/src/arrays/listview/rebuild.rs b/vortex-array/src/arrays/listview/rebuild.rs index ef1aeed633f..5a62135ae3a 100644 --- a/vortex-array/src/arrays/listview/rebuild.rs +++ b/vortex-array/src/arrays/listview/rebuild.rs @@ -311,8 +311,7 @@ impl ListViewArray { new_offsets.push(n_elements); new_sizes.push(size); - new_elements_builder - .extend_from_array(&elements_canonical.slice(start..stop)?, &mut ctx)?; + new_elements_builder.extend_from_array(&elements_canonical.slice(start..stop)?, ctx)?; n_elements += num_traits::cast(size).vortex_expect("Cast failed"); }