Use cursor copies for PiecewiseSequence slices

Signed-off-by: Daniel King <dan@spiraldb.com>

Author: danking

vortex-array/src/arrays/decimal/compute/take.rs

@@ -18,6 +18,7 @@ use crate::arrays::PiecewiseSequence;
 use crate::arrays::PrimitiveArray;
 use crate::arrays::dict::TakeExecute;
 use crate::arrays::piecewise_sequence::constant_unsigned_usize;
+use crate::arrays::piecewise_sequence::copy_slice_to_spare;
 use crate::arrays::piecewise_sequence::maybe_contiguous_slices;
 use crate::dtype::IntegerPType;
 use crate::dtype::NativeDecimalType;
@@ -199,11 +200,19 @@ where
     );
 
     let mut result = BufferMut::<T>::with_capacity(output_len);
+    let mut cursor = 0usize;
     for &start in starts {
         let start = start.as_();
-        result.extend_from_slice(&values[start..][..length]);
+        cursor = copy_slice_to_spare(&mut result, cursor, &values[start..][..length], output_len)?;
     }
 
+    vortex_ensure!(
+        cursor == output_len,
+        "PiecewiseSequenceArray expanded length {cursor} does not match declared length {output_len}"
+    );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_len`, and `cursor ==
+    // output_len` proves all slots were initialized.
+    unsafe { result.set_len(output_len) };
     Ok(result.freeze())
 }
 
@@ -219,17 +228,21 @@ where
     T: NativeDecimalType,
 {
     let mut result = BufferMut::<T>::with_capacity(output_len);
+    let mut cursor = 0usize;
     for (&start, &length) in starts.iter().zip_eq(lengths) {
         let start = start.as_();
         let length = length.as_();
-        result.extend_from_slice(&values[start..][..length]);
+        cursor = copy_slice_to_spare(&mut result, cursor, &values[start..][..length], output_len)?;
     }
 
     vortex_ensure!(
-        result.len() == output_len,
+        cursor == output_len,
         "PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
-        result.len()
+        cursor
     );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_len`, and `cursor ==
+    // output_len` proves all slots were initialized.
+    unsafe { result.set_len(output_len) };
     Ok(result.freeze())
 }
 

vortex-array/src/arrays/piecewise_sequence/mod.rs

@@ -8,6 +8,8 @@
 //! intended for take operations that can gather regular runs without materializing one index per
 //! element.
 
+use std::ptr;
+
 use itertools::Itertools;
 use num_traits::AsPrimitive;
 use vortex_buffer::BufferMut;
@@ -92,6 +94,40 @@ pub(crate) fn is_constant_one(multipliers: &ArrayRef) -> bool {
     )
 }
 
+pub(crate) fn copy_slice_to_spare<T: Copy>(
+    buffer: &mut BufferMut<T>,
+    cursor: usize,
+    source: &[T],
+    output_len: usize,
+) -> VortexResult<usize> {
+    let end = cursor
+        .checked_add(source.len())
+        .ok_or_else(|| vortex_err!("slice copy output length overflows usize"))?;
+    vortex_ensure!(
+        end <= output_len,
+        "slice copy length {end} exceeds declared output length {output_len}"
+    );
+    vortex_ensure!(
+        buffer.is_empty(),
+        "slice copy buffer already has {} initialized values",
+        buffer.len()
+    );
+    vortex_ensure!(
+        output_len <= buffer.capacity(),
+        "slice copy output length {output_len} exceeds buffer capacity {}",
+        buffer.capacity()
+    );
+
+    // SAFETY: the checks above prove `cursor..end` is inside the spare capacity of an
+    // uninitialized buffer allocated for at least `output_len` values.
+    let dst = unsafe { buffer.spare_capacity_mut().get_unchecked_mut(cursor..end) };
+    // SAFETY: `dst` has exactly `source.len()` writable slots and does not overlap with `source`.
+    unsafe {
+        ptr::copy_nonoverlapping(source.as_ptr(), dst.as_mut_ptr().cast(), source.len());
+    }
+    Ok(end)
+}
+
 pub(crate) fn constant_unsigned_usize(array: &ConstantArray) -> usize {
     let pvalue = array
         .scalar()

vortex-array/src/arrays/primitive/compute/take/mod.rs

@@ -25,6 +25,7 @@ use crate::arrays::Primitive;
 use crate::arrays::PrimitiveArray;
 use crate::arrays::dict::TakeExecute;
 use crate::arrays::piecewise_sequence::constant_unsigned_usize;
+use crate::arrays::piecewise_sequence::copy_slice_to_spare;
 use crate::arrays::piecewise_sequence::maybe_contiguous_slices;
 use crate::builtins::ArrayBuiltins;
 use crate::dtype::DType;
@@ -245,17 +246,21 @@ where
     L: UnsignedPType,
 {
     let mut values = BufferMut::<T>::with_capacity(output_len);
+    let mut cursor = 0usize;
     for (&start, &length) in starts.iter().zip_eq(lengths) {
         let start = start.as_();
         let length = length.as_();
-        values.extend_from_slice(&source[start..][..length]);
+        cursor = copy_slice_to_spare(&mut values, cursor, &source[start..][..length], output_len)?;
     }
 
     vortex_ensure!(
-        values.len() == output_len,
+        cursor == output_len,
         "PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
-        values.len()
+        cursor
     );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_len`, and `cursor ==
+    // output_len` proves all slots were initialized.
+    unsafe { values.set_len(output_len) };
     Ok(values.freeze())
 }
 
@@ -312,11 +317,19 @@ where
     );
 
     let mut values = BufferMut::<T>::with_capacity(output_len);
+    let mut cursor = 0usize;
     for &start in starts {
         let start = start.as_();
-        values.extend_from_slice(&source[start..][..length]);
+        cursor = copy_slice_to_spare(&mut values, cursor, &source[start..][..length], output_len)?;
     }
 
+    vortex_ensure!(
+        cursor == output_len,
+        "PiecewiseSequenceArray expanded length {cursor} does not match declared length {output_len}"
+    );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_len`, and `cursor ==
+    // output_len` proves all slots were initialized.
+    unsafe { values.set_len(output_len) };
     Ok(values.freeze())
 }
 

vortex-array/src/arrays/varbin/compute/take.rs

@@ -22,6 +22,7 @@ use crate::arrays::VarBin;
 use crate::arrays::VarBinArray;
 use crate::arrays::dict::TakeExecute;
 use crate::arrays::piecewise_sequence::constant_unsigned_usize;
+use crate::arrays::piecewise_sequence::copy_slice_to_spare;
 use crate::arrays::piecewise_sequence::maybe_contiguous_slices;
 use crate::arrays::primitive::PrimitiveArrayExt;
 use crate::arrays::varbin::VarBinArrayExt;
@@ -474,6 +475,7 @@ where
     }
 
     let mut new_data = ByteBufferMut::with_capacity(output_bytes);
+    let mut cursor = 0usize;
     for &start in starts {
         let start = start.as_();
         if length == 0 {
@@ -483,8 +485,20 @@ where
         let offset_range = &offsets[start..][..=length];
         let byte_start = offset_range[0].as_();
         let byte_end = offset_range[length].as_();
-        new_data.extend_from_slice(&data[byte_start..][..byte_end - byte_start]);
+        cursor = copy_slice_to_spare(
+            &mut new_data,
+            cursor,
+            &data[byte_start..][..byte_end - byte_start],
+            output_bytes,
+        )?;
     }
+    vortex_ensure!(
+        cursor == output_bytes,
+        "VarBin byte copy length {cursor} does not match computed byte length {output_bytes}"
+    );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_bytes`, and `cursor ==
+    // output_bytes` proves all bytes were initialized.
+    unsafe { new_data.set_len(output_bytes) };
 
     let offsets = PrimitiveArray::new(new_offsets.freeze(), Validity::NonNullable)
         .reinterpret_cast(out_offset_ptype)
@@ -551,6 +565,7 @@ where
     );
 
     let mut new_data = ByteBufferMut::with_capacity(output_bytes);
+    let mut cursor = 0usize;
     for (&start, &length) in starts.iter().zip_eq(lengths) {
         let start = start.as_();
         let length = length.as_();
@@ -561,8 +576,20 @@ where
         let offset_range = &offsets[start..][..=length];
         let byte_start = offset_range[0].as_();
         let byte_end = offset_range[length].as_();
-        new_data.extend_from_slice(&data[byte_start..][..byte_end - byte_start]);
+        cursor = copy_slice_to_spare(
+            &mut new_data,
+            cursor,
+            &data[byte_start..][..byte_end - byte_start],
+            output_bytes,
+        )?;
     }
+    vortex_ensure!(
+        cursor == output_bytes,
+        "VarBin byte copy length {cursor} does not match computed byte length {output_bytes}"
+    );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_bytes`, and `cursor ==
+    // output_bytes` proves all bytes were initialized.
+    unsafe { new_data.set_len(output_bytes) };
 
     let offsets = PrimitiveArray::new(new_offsets.freeze(), Validity::NonNullable)
         .reinterpret_cast(out_offset_ptype)

vortex-array/src/arrays/varbinview/compute/take.rs

@@ -24,6 +24,7 @@ use crate::arrays::VarBinView;
 use crate::arrays::VarBinViewArray;
 use crate::arrays::dict::TakeExecute;
 use crate::arrays::piecewise_sequence::constant_unsigned_usize;
+use crate::arrays::piecewise_sequence::copy_slice_to_spare;
 use crate::arrays::piecewise_sequence::maybe_contiguous_slices;
 use crate::arrays::varbinview::BinaryView;
 use crate::buffer::BufferHandle;
@@ -174,11 +175,19 @@ where
     );
 
     let mut views = BufferMut::<BinaryView>::with_capacity(output_len);
+    let mut cursor = 0usize;
     for &start in starts {
         let start = start.as_();
-        views.extend_from_slice(&source[start..][..length]);
+        cursor = copy_slice_to_spare(&mut views, cursor, &source[start..][..length], output_len)?;
     }
 
+    vortex_ensure!(
+        cursor == output_len,
+        "PiecewiseSequenceArray expanded length {cursor} does not match declared length {output_len}"
+    );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_len`, and `cursor ==
+    // output_len` proves all slots were initialized.
+    unsafe { views.set_len(output_len) };
     Ok(views.freeze())
 }
 
@@ -193,17 +202,21 @@ where
     L: UnsignedPType,
 {
     let mut views = BufferMut::<BinaryView>::with_capacity(output_len);
+    let mut cursor = 0usize;
     for (&start, &length) in starts.iter().zip_eq(lengths) {
         let start = start.as_();
         let length = length.as_();
-        views.extend_from_slice(&source[start..][..length]);
+        cursor = copy_slice_to_spare(&mut views, cursor, &source[start..][..length], output_len)?;
     }
 
     vortex_ensure!(
-        views.len() == output_len,
+        cursor == output_len,
         "PiecewiseSequenceArray expanded length {} does not match declared length {output_len}",
-        views.len()
+        cursor
     );
+    // SAFETY: `copy_slice_to_spare` checked every write against `output_len`, and `cursor ==
+    // output_len` proves all slots were initialized.
+    unsafe { views.set_len(output_len) };
     Ok(views.freeze())
 }