fixes

Signed-off-by: Andrew Duffy <andrew@a10y.dev>

Author: a10y

vortex-array/public-api.lock

@@ -3304,9 +3304,9 @@ pub fn vortex_array::arrays::patched::Patched::deserialize(bytes: &[u8], _dtype:
 
 pub fn vortex_array::arrays::patched::Patched::dtype(array: &Self::Array) -> &vortex_array::dtype::DType
 
-pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<Self::Array>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
+pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<vortex_array::vtable::Array<Self>>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
 
-pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
 pub fn vortex_array::arrays::patched::Patched::id(&self) -> vortex_array::vtable::ArrayId
 
@@ -3318,11 +3318,11 @@ pub fn vortex_array::arrays::patched::Patched::nbuffers(_array: &Self::Array) ->
 
 pub fn vortex_array::arrays::patched::Patched::nchildren(_array: &Self::Array) -> usize
 
-pub fn vortex_array::arrays::patched::Patched::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce(array: &vortex_array::vtable::Array<Self>) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
+pub fn vortex_array::arrays::patched::Patched::serialize(metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
 
 pub fn vortex_array::arrays::patched::Patched::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
 
@@ -7152,9 +7152,9 @@ pub fn vortex_array::arrays::patched::Patched::deserialize(bytes: &[u8], _dtype:
 
 pub fn vortex_array::arrays::patched::Patched::dtype(array: &Self::Array) -> &vortex_array::dtype::DType
 
-pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<Self::Array>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
+pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<vortex_array::vtable::Array<Self>>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
 
-pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
 pub fn vortex_array::arrays::patched::Patched::id(&self) -> vortex_array::vtable::ArrayId
 
@@ -7166,11 +7166,11 @@ pub fn vortex_array::arrays::patched::Patched::nbuffers(_array: &Self::Array) ->
 
 pub fn vortex_array::arrays::patched::Patched::nchildren(_array: &Self::Array) -> usize
 
-pub fn vortex_array::arrays::patched::Patched::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce(array: &vortex_array::vtable::Array<Self>) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
+pub fn vortex_array::arrays::patched::Patched::serialize(metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
 
 pub fn vortex_array::arrays::patched::Patched::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
 
@@ -22166,9 +22166,9 @@ pub fn vortex_array::arrays::patched::Patched::deserialize(bytes: &[u8], _dtype:
 
 pub fn vortex_array::arrays::patched::Patched::dtype(array: &Self::Array) -> &vortex_array::dtype::DType
 
-pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<Self::Array>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
+pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<vortex_array::vtable::Array<Self>>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
 
-pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
 pub fn vortex_array::arrays::patched::Patched::id(&self) -> vortex_array::vtable::ArrayId
 
@@ -22180,11 +22180,11 @@ pub fn vortex_array::arrays::patched::Patched::nbuffers(_array: &Self::Array) ->
 
 pub fn vortex_array::arrays::patched::Patched::nchildren(_array: &Self::Array) -> usize
 
-pub fn vortex_array::arrays::patched::Patched::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce(array: &vortex_array::vtable::Array<Self>) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
+pub fn vortex_array::arrays::patched::Patched::serialize(metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
 
 pub fn vortex_array::arrays::patched::Patched::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
 
@@ -23522,9 +23522,9 @@ pub fn vortex_array::arrays::patched::Patched::deserialize(bytes: &[u8], _dtype:
 
 pub fn vortex_array::arrays::patched::Patched::dtype(array: &Self::Array) -> &vortex_array::dtype::DType
 
-pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<Self::Array>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
+pub fn vortex_array::arrays::patched::Patched::execute(array: alloc::sync::Arc<vortex_array::vtable::Array<Self>>, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<vortex_array::ExecutionResult>
 
-pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::execute_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
 pub fn vortex_array::arrays::patched::Patched::id(&self) -> vortex_array::vtable::ArrayId
 
@@ -23536,11 +23536,11 @@ pub fn vortex_array::arrays::patched::Patched::nbuffers(_array: &Self::Array) ->
 
 pub fn vortex_array::arrays::patched::Patched::nchildren(_array: &Self::Array) -> usize
 
-pub fn vortex_array::arrays::patched::Patched::reduce(array: &Self::Array) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce(array: &vortex_array::vtable::Array<Self>) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &Self::Array, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
+pub fn vortex_array::arrays::patched::Patched::reduce_parent(array: &vortex_array::vtable::Array<Self>, parent: &vortex_array::ArrayRef, child_idx: usize) -> vortex_error::VortexResult<core::option::Option<vortex_array::ArrayRef>>
 
-pub fn vortex_array::arrays::patched::Patched::serialize(_metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
+pub fn vortex_array::arrays::patched::Patched::serialize(metadata: Self::Metadata) -> vortex_error::VortexResult<core::option::Option<alloc::vec::Vec<u8>>>
 
 pub fn vortex_array::arrays::patched::Patched::stats(array: &Self::Array) -> vortex_array::stats::StatsSetRef<'_>
 

vortex-array/src/arrays/patched/compute/compare.rs

@@ -74,78 +74,34 @@ impl CompareKernel for Patched {
                 .as_::<V>()
                 .vortex_expect("compare constant not null");
 
+            let apply_patches = ApplyPatches {
+                bits: &mut bits,
+                offset,
+                n_lanes,
+                lane_offsets,
+                indices,
+                values,
+                constant,
+            };
+
             match operator {
                 CompareOperator::Eq => {
-                    apply::<V, _>(
-                        &mut bits,
-                        offset,
-                        n_lanes,
-                        lane_offsets,
-                        indices,
-                        values,
-                        constant,
-                        |l, r| NativeValue(l) == NativeValue(r),
-                    )?;
+                    apply_patches.apply(|l, r| NativeValue(l) == NativeValue(r))?;
                 }
                 CompareOperator::NotEq => {
-                    apply::<V, _>(
-                        &mut bits,
-                        offset,
-                        n_lanes,
-                        lane_offsets,
-                        indices,
-                        values,
-                        constant,
-                        |l, r| NativeValue(l) != NativeValue(r),
-                    )?;
+                    apply_patches.apply(|l, r| NativeValue(l) != NativeValue(r))?;
                 }
                 CompareOperator::Gt => {
-                    apply::<V, _>(
-                        &mut bits,
-                        n_lanes,
-                        offset,
-                        lane_offsets,
-                        indices,
-                        values,
-                        constant,
-                        |l, r| NativeValue(l) > NativeValue(r),
-                    )?;
+                    apply_patches.apply(|l, r| NativeValue(l) > NativeValue(r))?;
                 }
                 CompareOperator::Gte => {
-                    apply::<V, _>(
-                        &mut bits,
-                        n_lanes,
-                        offset,
-                        lane_offsets,
-                        indices,
-                        values,
-                        constant,
-                        |l, r| NativeValue(l) >= NativeValue(r),
-                    )?;
+                    apply_patches.apply(|l, r| NativeValue(l) >= NativeValue(r))?;
                 }
                 CompareOperator::Lt => {
-                    apply::<V, _>(
-                        &mut bits,
-                        n_lanes,
-                        offset,
-                        lane_offsets,
-                        indices,
-                        values,
-                        constant,
-                        |l, r| NativeValue(l) < NativeValue(r),
-                    )?;
+                    apply_patches.apply(|l, r| NativeValue(l) < NativeValue(r))?;
                 }
                 CompareOperator::Lte => {
-                    apply::<V, _>(
-                        &mut bits,
-                        n_lanes,
-                        offset,
-                        lane_offsets,
-                        indices,
-                        values,
-                        constant,
-                        |l, r| NativeValue(l) <= NativeValue(r),
-                    )?;
+                    apply_patches.apply(|l, r| NativeValue(l) <= NativeValue(r))?;
                 }
             }
         });
@@ -155,45 +111,47 @@ impl CompareKernel for Patched {
     }
 }
 
-#[allow(clippy::too_many_arguments)]
-fn apply<V: NativePType, F>(
-    bits: &mut BitBufferMut,
+struct ApplyPatches<'a, V: NativePType> {
+    bits: &'a mut BitBufferMut,
     offset: usize,
     n_lanes: usize,
-    lane_offsets: &[u32],
-    indices: &[u16],
-    values: &[V],
+    lane_offsets: &'a [u32],
+    indices: &'a [u16],
+    values: &'a [V],
     constant: V,
-    cmp: F,
-) -> VortexResult<()>
-where
-    F: Fn(V, V) -> bool,
-{
-    for index in 0..(lane_offsets.len() - 1) {
-        let chunk = index / n_lanes;
-
-        let lane_start = lane_offsets[index] as usize;
-        let lane_end = lane_offsets[index + 1] as usize;
-
-        for (&patch_index, &patch_value) in std::iter::zip(
-            &indices[lane_start..lane_end],
-            &values[lane_start..lane_end],
-        ) {
-            let bit_index = chunk * 1024 + patch_index as usize;
-            // Skip any indices < the offset.
-            if bit_index < offset {
-                continue;
-            }
-            let bit_index = bit_index - offset;
-            if cmp(patch_value, constant) {
-                bits.set(bit_index)
-            } else {
-                bits.unset(bit_index)
+}
+
+impl<V: NativePType> ApplyPatches<'_, V> {
+    fn apply<F>(self, cmp: F) -> VortexResult<()>
+    where
+        F: Fn(V, V) -> bool,
+    {
+        for index in 0..(self.lane_offsets.len() - 1) {
+            let chunk = index / self.n_lanes;
+
+            let lane_start = self.lane_offsets[index] as usize;
+            let lane_end = self.lane_offsets[index + 1] as usize;
+
+            for (&patch_index, &patch_value) in std::iter::zip(
+                &self.indices[lane_start..lane_end],
+                &self.values[lane_start..lane_end],
+            ) {
+                let bit_index = chunk * 1024 + patch_index as usize;
+                // Skip any indices < the offset.
+                if bit_index < self.offset {
+                    continue;
+                }
+                let bit_index = bit_index - self.offset;
+                if cmp(patch_value, self.constant) {
+                    self.bits.set(bit_index)
+                } else {
+                    self.bits.unset(bit_index)
+                }
             }
         }
-    }
 
-    Ok(())
+        Ok(())
+    }
 }
 
 #[cfg(test)]

vortex-array/src/arrays/patched/vtable/mod.rs

@@ -46,10 +46,11 @@ use crate::serde::ArrayChildren;
 use crate::stats::ArrayStats;
 use crate::stats::StatsSetRef;
 use crate::vtable;
+use crate::vtable::Array;
+use crate::vtable::ArrayId;
 use crate::vtable::VTable;
 use crate::vtable::ValidityChild;
 use crate::vtable::ValidityVTableFromChild;
-use crate::vtable::{Array, ArrayId};
 
 vtable!(Patched);
 
@@ -265,8 +266,8 @@ impl VTable for Patched {
 
     fn with_children(array: &mut Self::Array, mut children: Vec<ArrayRef>) -> VortexResult<()> {
         vortex_ensure!(
-            children.len() == 2,
-            "PatchedArray must have exactly 2 children"
+            children.len() == 3,
+            "PatchedArray must have exactly 3 children"
         );
 
         array.inner = children.remove(0);
@@ -381,9 +382,7 @@ mod tests {
     use crate::arrays::PrimitiveArray;
     use crate::assert_arrays_eq;
     use crate::builders::builder_with_capacity;
-    use crate::dtype::Nullability;
     use crate::patches::Patches;
-    use crate::scalar::Scalar;
     use crate::validity::Validity;
 
     #[test]
@@ -419,43 +418,6 @@ mod tests {
         assert_eq!(executed, expected.freeze());
     }
 
-    #[test]
-    fn test_scalar_at() {
-        let values = buffer![0u16; 1024].into_array();
-        let patches = Patches::new(
-            1024,
-            0,
-            buffer![1u32, 2, 3].into_array(),
-            buffer![1u16; 3].into_array(),
-            None,
-        )
-        .unwrap();
-
-        let session = VortexSession::empty();
-        let mut ctx = ExecutionCtx::new(session);
-
-        let array = PatchedArray::from_array_and_patches(values, &patches, &mut ctx)
-            .unwrap()
-            .into_array();
-
-        assert_eq!(
-            array.scalar_at(0).unwrap(),
-            Scalar::primitive(0u16, Nullability::NonNullable)
-        );
-        assert_eq!(
-            array.scalar_at(1).unwrap(),
-            Scalar::primitive(1u16, Nullability::NonNullable)
-        );
-        assert_eq!(
-            array.scalar_at(2).unwrap(),
-            Scalar::primitive(1u16, Nullability::NonNullable)
-        );
-        assert_eq!(
-            array.scalar_at(3).unwrap(),
-            Scalar::primitive(1u16, Nullability::NonNullable)
-        );
-    }
-
     #[test]
     fn test_append_to_builder_non_nullable() {
         let values = PrimitiveArray::new(buffer![0u16; 1024], Validity::NonNullable).into_array();