diff --git a/encodings/alp/src/alp/array.rs b/encodings/alp/src/alp/array.rs index 6127c3cbe6b..a5bc0a50b12 100644 --- a/encodings/alp/src/alp/array.rs +++ b/encodings/alp/src/alp/array.rs @@ -172,7 +172,6 @@ impl VTable for ALP { let array = require_child!(array, array.encoded(), ENCODED_SLOT => Primitive); require_patches!( array, - array.patches(), PATCH_INDICES_SLOT, PATCH_VALUES_SLOT, PATCH_CHUNK_OFFSETS_SLOT diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index 49f65f6282d..f75727cba99 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -228,7 +228,6 @@ impl VTable for ALPRD { let array = require_child!(array, array.right_parts(), 1 => Primitive); require_patches!( array, - array.left_parts_patches(), LP_PATCH_INDICES_SLOT, LP_PATCH_VALUES_SLOT, LP_PATCH_CHUNK_OFFSETS_SLOT diff --git a/encodings/fastlanes/src/bitpacking/vtable/mod.rs b/encodings/fastlanes/src/bitpacking/vtable/mod.rs index 0898ecece50..61670731f5c 100644 --- a/encodings/fastlanes/src/bitpacking/vtable/mod.rs +++ b/encodings/fastlanes/src/bitpacking/vtable/mod.rs @@ -5,7 +5,6 @@ use std::hash::Hash; use std::hash::Hasher; use prost::Message; -use vortex_array::AnyCanonical; use vortex_array::Array; use vortex_array::ArrayEq; use vortex_array::ArrayHash; @@ -283,17 +282,11 @@ impl VTable for BitPacked { fn execute(array: Array, ctx: &mut ExecutionCtx) -> VortexResult { require_patches!( array, - array.patches(), PATCH_INDICES_SLOT, PATCH_VALUES_SLOT, PATCH_CHUNK_OFFSETS_SLOT ); - let validity = array.validity()?; - require_validity!( - array, - &validity, - VALIDITY_SLOT => AnyCanonical - ); + require_validity!(array, VALIDITY_SLOT); Ok(ExecutionResult::done( unpack_array(array.as_view(), ctx)?.into_array(), diff --git a/vortex-array/public-api.lock b/vortex-array/public-api.lock index 687b9d33fef..a448595c58d 100644 --- a/vortex-array/public-api.lock +++ b/vortex-array/public-api.lock @@ -3358,7 +3358,7 @@ pub fn vortex_array::arrays::patched::Patched::child_name(array: vortex_array::A pub fn vortex_array::arrays::patched::Patched::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::patched::Patched::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -4098,7 +4098,7 @@ pub fn vortex_array::arrays::slice::Slice::child_name(array: vortex_array::Array pub fn vortex_array::arrays::slice::Slice::deserialize(&self, _dtype: &vortex_array::dtype::DType, _len: usize, _metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::slice::Slice::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -6198,7 +6198,7 @@ pub fn vortex_array::arrays::patched::Patched::child_name(array: vortex_array::A pub fn vortex_array::arrays::patched::Patched::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::patched::Patched::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -6496,7 +6496,7 @@ pub fn vortex_array::arrays::slice::Slice::child_name(array: vortex_array::Array pub fn vortex_array::arrays::slice::Slice::deserialize(&self, _dtype: &vortex_array::dtype::DType, _len: usize, _metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::slice::Slice::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -19856,7 +19856,7 @@ pub fn vortex_array::arrays::patched::Patched::child_name(array: vortex_array::A pub fn vortex_array::arrays::patched::Patched::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::patched::Patched::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -19940,7 +19940,7 @@ pub fn vortex_array::arrays::slice::Slice::child_name(array: vortex_array::Array pub fn vortex_array::arrays::slice::Slice::deserialize(&self, _dtype: &vortex_array::dtype::DType, _len: usize, _metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::slice::Slice::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -20872,7 +20872,7 @@ pub fn vortex_array::arrays::patched::Patched::child_name(array: vortex_array::A pub fn vortex_array::arrays::patched::Patched::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::patched::Patched::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -20956,7 +20956,7 @@ pub fn vortex_array::arrays::slice::Slice::child_name(array: vortex_array::Array pub fn vortex_array::arrays::slice::Slice::deserialize(&self, _dtype: &vortex_array::dtype::DType, _len: usize, _metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::slice::Slice::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -23576,7 +23576,7 @@ pub fn vortex_array::arrays::patched::Patched::child_name(array: vortex_array::A pub fn vortex_array::arrays::patched::Patched::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::patched::Patched::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -23660,7 +23660,7 @@ pub fn vortex_array::arrays::slice::Slice::child_name(array: vortex_array::Array pub fn vortex_array::arrays::slice::Slice::deserialize(&self, _dtype: &vortex_array::dtype::DType, _len: usize, _metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::slice::Slice::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -24840,7 +24840,7 @@ pub fn vortex_array::arrays::patched::Patched::child_name(array: vortex_array::A pub fn vortex_array::arrays::patched::Patched::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::patched::Patched::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::patched::Patched::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -24924,7 +24924,7 @@ pub fn vortex_array::arrays::slice::Slice::child_name(array: vortex_array::Array pub fn vortex_array::arrays::slice::Slice::deserialize(&self, _dtype: &vortex_array::dtype::DType, _len: usize, _metadata: &[u8], _buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, _session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::slice::Slice::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::slice::Slice::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> diff --git a/vortex-array/src/array/vtable/mod.rs b/vortex-array/src/array/vtable/mod.rs index 4a12e80fa99..930be7a279c 100644 --- a/vortex-array/src/array/vtable/mod.rs +++ b/vortex-array/src/array/vtable/mod.rs @@ -165,9 +165,16 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug { /// Execute this array by returning an [`ExecutionResult`]. /// - /// Instead of recursively executing children, implementations should return - /// [`ExecutionResult::execute_slot`] to request that the scheduler execute a slot first, - /// or [`ExecutionResult::done`] when the encoding can produce a result directly. + /// Execution is **iterative**, not recursive. Instead of recursively executing children, + /// implementations should return [`ExecutionResult::execute_slot`] to request that the + /// scheduler execute a slot first, or [`ExecutionResult::done`] when the encoding can + /// produce a result directly. + /// + /// For good examples of this pattern, see: + /// - [`Dict::execute`](crate::arrays::dict::vtable::Dict::execute) — demonstrates + /// requiring children via `require_child!` and producing a result once they are canonical. + /// - `BitPacked::execute` (in `vortex-fastlanes`) — demonstrates requiring patches and + /// validity via `require_patches!`/`require_validity!`. /// /// Array execution is designed such that repeated execution of an array will eventually /// converge to a canonical representation. Implementations of this function should therefore diff --git a/vortex-array/src/arrays/filter/vtable.rs b/vortex-array/src/arrays/filter/vtable.rs index 7490bcd8b41..516a8dbe1ad 100644 --- a/vortex-array/src/arrays/filter/vtable.rs +++ b/vortex-array/src/arrays/filter/vtable.rs @@ -11,9 +11,11 @@ use vortex_error::vortex_panic; use vortex_mask::Mask; use vortex_session::VortexSession; +use crate::AnyCanonical; use crate::ArrayEq; use crate::ArrayHash; use crate::ArrayRef; +use crate::Canonical; use crate::IntoArray; use crate::Precision; use crate::array::Array; @@ -34,6 +36,7 @@ use crate::buffer::BufferHandle; use crate::dtype::DType; use crate::executor::ExecutionCtx; use crate::executor::ExecutionResult; +use crate::require_child; use crate::scalar::Scalar; use crate::serde::ArrayChildren; use crate::validity::Validity; @@ -142,14 +145,19 @@ impl VTable for Filter { if let Some(canonical) = execute_filter_fast_paths(array.as_view(), ctx)? { return Ok(ExecutionResult::done(canonical)); } - let Mask::Values(mask_values) = &array.mask else { - unreachable!("`execute_filter_fast_paths` handles AllTrue and AllFalse") + let mask_values = match &array.mask { + Mask::Values(v) => v.clone(), + _ => unreachable!("`execute_filter_fast_paths` handles AllTrue and AllFalse"), }; + let array = require_child!(array, array.child(), CHILD_SLOT => AnyCanonical); + // We rely on the optimization pass that runs prior to this execution for filter pushdown, // so now we can just execute the filter without worrying. + // TODO(joe): fix the ownership of AnyCanonical + let child = Canonical::from(array.child().as_::()); Ok(ExecutionResult::done( - execute_filter(array.child().clone().execute(ctx)?, mask_values).into_array(), + execute_filter(child, &mask_values).into_array(), )) } diff --git a/vortex-array/src/arrays/masked/vtable/mod.rs b/vortex-array/src/arrays/masked/vtable/mod.rs index 45b8a48cb2d..f4924e28ea7 100644 --- a/vortex-array/src/arrays/masked/vtable/mod.rs +++ b/vortex-array/src/arrays/masked/vtable/mod.rs @@ -13,6 +13,7 @@ use vortex_error::vortex_ensure; use vortex_error::vortex_panic; use vortex_session::VortexSession; +use crate::AnyCanonical; use crate::ArrayEq; use crate::ArrayHash; use crate::ArrayRef; @@ -29,12 +30,15 @@ use crate::arrays::masked::MaskedArrayExt; use crate::arrays::masked::MaskedData; use crate::arrays::masked::array::CHILD_SLOT; use crate::arrays::masked::array::SLOT_NAMES; +use crate::arrays::masked::array::VALIDITY_SLOT; use crate::arrays::masked::compute::rules::PARENT_RULES; use crate::arrays::masked::mask_validity_canonical; use crate::buffer::BufferHandle; use crate::dtype::DType; use crate::executor::ExecutionCtx; use crate::executor::ExecutionResult; +use crate::require_child; +use crate::require_validity; use crate::scalar::Scalar; use crate::serde::ArrayChildren; use crate::validity::Validity; @@ -166,7 +170,10 @@ impl VTable for Masked { // While we could manually convert the dtype, `mask_validity_canonical` is already O(1) for // `AllTrue` masks (no data copying), so there's no benefit. - let child = array.child().clone().execute::(ctx)?; + let array = require_child!(array, array.child(), CHILD_SLOT => AnyCanonical); + require_validity!(array, VALIDITY_SLOT); + + let child = Canonical::from(array.child().as_::()); Ok(ExecutionResult::done( mask_validity_canonical(child, &validity_mask, ctx)?.into_array(), )) diff --git a/vortex-array/src/arrays/patched/vtable/mod.rs b/vortex-array/src/arrays/patched/vtable/mod.rs index 5b8f900c7f0..e8e2aacebd0 100644 --- a/vortex-array/src/arrays/patched/vtable/mod.rs +++ b/vortex-array/src/arrays/patched/vtable/mod.rs @@ -31,10 +31,15 @@ use crate::array::ArrayView; use crate::array::VTable; use crate::array::ValidityChild; use crate::array::ValidityVTableFromChild; +use crate::arrays::Primitive; use crate::arrays::PrimitiveArray; use crate::arrays::patched::PatchedArrayExt; use crate::arrays::patched::PatchedData; +use crate::arrays::patched::array::INDICES_SLOT; +use crate::arrays::patched::array::INNER_SLOT; +use crate::arrays::patched::array::LANE_OFFSETS_SLOT; use crate::arrays::patched::array::SLOT_NAMES; +use crate::arrays::patched::array::VALUES_SLOT; use crate::arrays::patched::compute::rules::PARENT_RULES; use crate::arrays::patched::vtable::kernels::PARENT_KERNELS; use crate::arrays::primitive::PrimitiveDataParts; @@ -45,6 +50,7 @@ use crate::dtype::DType; use crate::dtype::NativePType; use crate::dtype::PType; use crate::match_each_native_ptype; +use crate::require_child; use crate::serde::ArrayChildren; /// A [`Patched`]-encoded Vortex array. @@ -242,12 +248,46 @@ impl VTable for Patched { SLOT_NAMES[idx].to_string() } - fn execute(array: Array, ctx: &mut ExecutionCtx) -> VortexResult { - let inner = array - .base_array() - .clone() - .execute::(ctx)? - .into_primitive(); + fn execute(array: Array, _ctx: &mut ExecutionCtx) -> VortexResult { + let array = require_child!(array, array.base_array(), INNER_SLOT => Primitive); + let array = require_child!(array, array.lane_offsets(), LANE_OFFSETS_SLOT => Primitive); + let array = require_child!(array, array.patch_indices(), INDICES_SLOT => Primitive); + let array = require_child!(array, array.patch_values(), VALUES_SLOT => Primitive); + + let len = array.len(); + let (data, mut slots) = match array.try_into_parts() { + Ok(parts) => (parts.data, parts.slots), + Err(array) => { + // Arc is shared, clone the fields out. + (array.data().clone(), array.slots().to_vec()) + } + }; + let PatchedData { n_lanes, offset } = data; + + let inner = slots[INNER_SLOT] + .take() + .vortex_expect("inner slot") + .try_downcast::() + .ok() + .vortex_expect("inner must be primitive"); + let lane_offsets = slots[LANE_OFFSETS_SLOT] + .take() + .vortex_expect("lane_offsets slot") + .try_downcast::() + .ok() + .vortex_expect("lane_offsets must be primitive"); + let indices = slots[INDICES_SLOT] + .take() + .vortex_expect("indices slot") + .try_downcast::() + .ok() + .vortex_expect("indices must be primitive"); + let values = slots[VALUES_SLOT] + .take() + .vortex_expect("values slot") + .try_downcast::() + .ok() + .vortex_expect("values must be primitive"); let PrimitiveDataParts { buffer, @@ -255,32 +295,14 @@ impl VTable for Patched { validity, } = inner.into_data_parts(); - let lane_offsets = array - .lane_offsets() - .clone() - .execute::(ctx)?; - let indices = array - .patch_indices() - .clone() - .execute::(ctx)?; - - // TODO(aduffy): add support for non-primitive PatchedArray patches application (?) - let values = array - .patch_values() - .clone() - .execute::(ctx)?; - let patched_values = match_each_native_ptype!(values.ptype(), |V| { - let offset = array.offset(); - let len = array.len(); - let mut output = Buffer::::from_byte_buffer(buffer.unwrap_host()).into_mut(); apply_patches_primitive::( &mut output, offset, len, - array.n_lanes(), + n_lanes, lane_offsets.as_slice::(), indices.as_slice::(), values.as_slice::(), diff --git a/vortex-array/src/arrays/slice/vtable.rs b/vortex-array/src/arrays/slice/vtable.rs index 72b5f943eb6..0d29d519806 100644 --- a/vortex-array/src/arrays/slice/vtable.rs +++ b/vortex-array/src/arrays/slice/vtable.rs @@ -18,8 +18,6 @@ use crate::AnyCanonical; use crate::ArrayEq; use crate::ArrayHash; use crate::ArrayRef; -use crate::Canonical; -use crate::IntoArray; use crate::Precision; use crate::array::Array; use crate::array::ArrayId; @@ -36,6 +34,7 @@ use crate::buffer::BufferHandle; use crate::dtype::DType; use crate::executor::ExecutionCtx; use crate::executor::ExecutionResult; +use crate::require_child; use crate::scalar::Scalar; use crate::serde::ArrayChildren; use crate::validity::Validity; @@ -141,21 +140,13 @@ impl VTable for Slice { vortex_bail!("Slice array is not serializable") } - fn execute(array: Array, ctx: &mut ExecutionCtx) -> VortexResult { - // Execute the child to get canonical form, then slice it - let Some(canonical) = array.child().as_opt::() else { - // If the child is not canonical, recurse. - return array - .child() - .clone() - .execute::(ctx)? - .slice(array.slice_range().clone()) - .map(ExecutionResult::done); - }; + fn execute(array: Array, _ctx: &mut ExecutionCtx) -> VortexResult { + let array = require_child!(array, array.child(), CHILD_SLOT => AnyCanonical); + debug_assert!(array.child().is_canonical()); // TODO(ngates): we should inline canonical slice logic here. - Canonical::from(canonical) - .into_array() + array + .child() .slice(array.range.clone()) .map(ExecutionResult::done) } diff --git a/vortex-array/src/compute/conformance/binary_numeric.rs b/vortex-array/src/compute/conformance/binary_numeric.rs index d55b65664b1..c36367746e7 100644 --- a/vortex-array/src/compute/conformance/binary_numeric.rs +++ b/vortex-array/src/compute/conformance/binary_numeric.rs @@ -126,9 +126,6 @@ where continue; }; - println!("result {}", result.display_tree()); - println!("result {}", result.display_values()); - let actual_values = to_vec_of_scalar(&result); // Check each element for overflow/underflow diff --git a/vortex-array/src/executor.rs b/vortex-array/src/executor.rs index 623410938d2..80581795996 100644 --- a/vortex-array/src/executor.rs +++ b/vortex-array/src/executor.rs @@ -488,55 +488,52 @@ macro_rules! require_opt_child { }; } -/// Require that all children of a [`Patches`](crate::patches::Patches) (indices, values, and -/// optionally chunk_offsets) are `Primitive`. If no patches are present, this is a no-op. +/// Require that patch slots (indices, values, and optionally chunk_offsets) are `Primitive`. +/// If no patches are present (slots are `None`), this is a no-op. /// /// Like [`require_opt_child!`], `$parent` is moved (not cloned) into the early-return path. /// /// ```ignore -/// require_patches!(array, array.patches(), PATCH_INDICES_SLOT, PATCH_VALUES_SLOT, PATCH_CHUNK_OFFSETS_SLOT); +/// require_patches!(array, PATCH_INDICES_SLOT, PATCH_VALUES_SLOT, PATCH_CHUNK_OFFSETS_SLOT); /// ``` #[macro_export] macro_rules! require_patches { - ($parent:expr, $patches:expr, $indices_slot:expr, $values_slot:expr, $chunk_offsets_slot:expr) => { - let __patches = $patches; + ($parent:expr, $indices_slot:expr, $values_slot:expr, $chunk_offsets_slot:expr) => { $crate::require_opt_child!( $parent, - __patches.as_ref().map(|p| p.indices()), + $parent.slots()[$indices_slot].as_ref(), $indices_slot => $crate::arrays::Primitive ); - let __patches = $patches; $crate::require_opt_child!( $parent, - __patches.as_ref().map(|p| p.values()), + $parent.slots()[$values_slot].as_ref(), $values_slot => $crate::arrays::Primitive ); - let __patches = $patches; $crate::require_opt_child!( $parent, - __patches.as_ref().and_then(|p| p.chunk_offsets().as_ref()), + $parent.slots()[$chunk_offsets_slot].as_ref(), $chunk_offsets_slot => $crate::arrays::Primitive ); }; } -/// Require that a [`Validity::Array`](crate::validity::Validity::Array) child matches `$M`. If validity is not array-backed -/// (e.g. `NonNullable` or `AllValid`), this is a no-op. If it is array-backed but does not -/// match `$M`, early-returns an [`ExecutionResult`] requesting execution of the validity slot. +/// Require that the validity slot is a [`Bool`](crate::arrays::Bool) array. If validity is not +/// array-backed (e.g. `NonNullable` or `AllValid`), this is a no-op. If it is array-backed but +/// not `Bool`, early-returns an [`ExecutionResult`] requesting execution of the validity slot. /// /// Like [`require_opt_child!`], `$parent` is moved (not cloned) into the early-return path. /// /// ```ignore -/// require_validity!(array, &array.validity, VALIDITY_SLOT => AnyCanonical); +/// require_validity!(array, VALIDITY_SLOT); /// ``` #[macro_export] macro_rules! require_validity { - ($parent:expr, $validity:expr, $idx:expr => $M:ty) => { - if let $crate::validity::Validity::Array(v) = $validity { - if !v.is::<$M>() { - return Ok($crate::ExecutionResult::execute_slot::<$M>($parent, $idx)); - } - } + ($parent:expr, $idx:expr) => { + $crate::require_opt_child!( + $parent, + $parent.slots()[$idx].as_ref(), + $idx => $crate::arrays::Bool + ); }; }