Cleanup fastlanes encodings crate (#7118)

* Remove dead code only used in tests
* Simplify signatures that needlessly return Results
* Add test coverage for bit_transpose functions
* Propagate patch offsets when compressing patches

---------

Signed-off-by: Robert Kruszewski <github@robertk.io>

Author: robert3005

encodings/fastlanes/public-api.lock

@@ -26,7 +26,7 @@ pub fn vortex_fastlanes::bitpack_compress::bitpack_primitive<T: vortex_array::dt
 
 pub fn vortex_fastlanes::bitpack_compress::bitpack_to_best_bit_width(array: &vortex_array::arrays::primitive::array::PrimitiveArray) -> vortex_error::VortexResult<vortex_fastlanes::BitPackedArray>
 
-pub unsafe fn vortex_fastlanes::bitpack_compress::bitpack_unchecked(parray: &vortex_array::arrays::primitive::array::PrimitiveArray, bit_width: u8) -> vortex_error::VortexResult<vortex_buffer::ByteBuffer>
+pub unsafe fn vortex_fastlanes::bitpack_compress::bitpack_unchecked(parray: &vortex_array::arrays::primitive::array::PrimitiveArray, bit_width: u8) -> vortex_buffer::ByteBuffer
 
 pub fn vortex_fastlanes::bitpack_compress::find_best_bit_width(ptype: vortex_array::dtype::ptype::PType, bit_width_freq: &[usize]) -> vortex_error::VortexResult<u8>
 
@@ -48,10 +48,6 @@ pub fn vortex_fastlanes::bitpack_decompress::unpack_single(array: &vortex_fastla
 
 pub unsafe fn vortex_fastlanes::bitpack_decompress::unpack_single_primitive<T: vortex_array::dtype::ptype::NativePType + fastlanes::bitpacking::BitPacking>(packed: &[T], bit_width: usize, index_to_decode: usize) -> T
 
-pub fn vortex_fastlanes::bitpack_decompress::unpack_to_primitive(array: &vortex_fastlanes::BitPackedArray) -> vortex_array::arrays::primitive::array::PrimitiveArray
-
-pub fn vortex_fastlanes::bitpack_decompress::unpack_to_primitive_typed<P: vortex_fastlanes::unpack_iter::BitPacked>(array: &vortex_fastlanes::BitPackedArray) -> vortex_array::arrays::primitive::array::PrimitiveArray
-
 pub mod vortex_fastlanes::unpack_iter
 
 pub struct vortex_fastlanes::unpack_iter::BitPackingStrategy
@@ -380,11 +376,9 @@ pub fn vortex_fastlanes::DeltaArray::len(&self) -> usize
 
 pub fn vortex_fastlanes::DeltaArray::offset(&self) -> usize
 
-pub fn vortex_fastlanes::DeltaArray::try_from_delta_compress_parts(bases: vortex_array::array::ArrayRef, deltas: vortex_array::array::ArrayRef) -> vortex_error::VortexResult<Self>
-
 pub fn vortex_fastlanes::DeltaArray::try_from_primitive_array(array: &vortex_array::arrays::primitive::array::PrimitiveArray, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult<Self>
 
-pub fn vortex_fastlanes::DeltaArray::try_new(bases: vortex_array::array::ArrayRef, deltas: vortex_array::array::ArrayRef, offset: usize, logical_len: usize) -> vortex_error::VortexResult<Self>
+pub fn vortex_fastlanes::DeltaArray::try_new(bases: vortex_array::array::ArrayRef, deltas: vortex_array::array::ArrayRef, offset: usize, len: usize) -> vortex_error::VortexResult<Self>
 
 impl vortex_fastlanes::DeltaArray
 

encodings/fastlanes/src/bit_transpose/validity.rs

@@ -37,7 +37,6 @@ pub fn transpose_validity(validity: &Validity, ctx: &mut ExecutionCtx) -> Vortex
     }
 }
 
-#[inline]
 pub fn transpose_bitbuffer(bits: BitBuffer) -> BitBuffer {
     let (offset, len, bytes) = bits.into_inner();
 
@@ -79,11 +78,10 @@ pub fn untranspose_validity(validity: &Validity, ctx: &mut ExecutionCtx) -> Vort
     }
 }
 
-#[inline]
 pub fn untranspose_bitbuffer(bits: BitBuffer) -> BitBuffer {
     assert!(
         bits.inner().len().is_multiple_of(128),
-        "Transpose BitBuffer must be 128-byte aligned"
+        "Transpose BitBuffer byte length must be a multiple of 128"
     );
     let (offset, len, bytes) = bits.into_inner();
     match bytes.try_into_mut() {
@@ -137,3 +135,58 @@ fn bits_op_with_copy<F: Fn(&[u8; 128], &mut [u8; 128])>(
         offset,
     )
 }
+
+#[cfg(test)]
+mod tests {
+    use vortex_buffer::BitBuffer;
+    use vortex_buffer::BitBufferMut;
+    use vortex_buffer::ByteBuffer;
+
+    use super::*;
+
+    fn make_validity_bits(num_bits: usize) -> BitBuffer {
+        let mut builder = BitBufferMut::with_capacity(num_bits);
+        for i in 0..num_bits {
+            builder.append(i % 3 != 0);
+        }
+        builder.freeze()
+    }
+
+    fn force_copy_path(bits: BitBuffer) -> (BitBuffer, ByteBuffer) {
+        let (offset, len, bytes) = bits.into_inner();
+        let extra_ref = bytes.clone();
+        (BitBuffer::new_with_offset(bytes, len, offset), extra_ref)
+    }
+
+    #[test]
+    fn transpose_padding_copy_produces_same_bits() {
+        let bits = make_validity_bits(500);
+        let transposed = transpose_bitbuffer(bits.clone());
+        assert_eq!(transposed.len(), 1024);
+        let untransposed = untranspose_bitbuffer(transposed);
+        assert_eq!(untransposed.slice(0..500), bits)
+    }
+
+    #[test]
+    fn transpose_inplace_and_copy_produce_same_bits() {
+        let bits = make_validity_bits(2048);
+
+        let inplace_result = transpose_bitbuffer(bits.clone());
+
+        let (bits_shared, _hold) = force_copy_path(bits);
+        let copy_result = transpose_bitbuffer(bits_shared);
+
+        assert_eq!(inplace_result.len(), copy_result.len());
+        assert_eq!(inplace_result, copy_result);
+    }
+
+    #[test]
+    fn transpose_validity_roundtrip_non_aligned() {
+        let original_len = 1500;
+        let bits = make_validity_bits(original_len);
+
+        let transposed = transpose_bitbuffer(bits.clone());
+        let roundtripped = untranspose_bitbuffer(transposed);
+        assert_eq!(bits, roundtripped.slice(0..original_len));
+    }
+}

encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs

@@ -65,7 +65,7 @@ pub fn bitpack_encode(
     }
 
     // SAFETY: we check that array only contains non-negative values.
-    let packed = unsafe { bitpack_unchecked(array, bit_width)? };
+    let packed = unsafe { bitpack_unchecked(array, bit_width) };
     let patches = (num_exceptions > 0)
         .then(|| gather_patches(array, bit_width, num_exceptions))
         .transpose()?
@@ -103,7 +103,7 @@ pub unsafe fn bitpack_encode_unchecked(
     bit_width: u8,
 ) -> VortexResult<BitPackedArray> {
     // SAFETY: non-negativity of input checked by caller.
-    let packed = unsafe { bitpack_unchecked(&array, bit_width)? };
+    let packed = unsafe { bitpack_unchecked(&array, bit_width) };
 
     // SAFETY: checked by bitpack_unchecked
     let bitpacked = unsafe {
@@ -135,15 +135,11 @@ pub unsafe fn bitpack_encode_unchecked(
 ///
 /// It is the caller's responsibility to ensure that `parray` is non-negative before calling
 /// this function.
-pub unsafe fn bitpack_unchecked(
-    parray: &PrimitiveArray,
-    bit_width: u8,
-) -> VortexResult<ByteBuffer> {
+pub unsafe fn bitpack_unchecked(parray: &PrimitiveArray, bit_width: u8) -> ByteBuffer {
     let parray = parray.reinterpret_cast(parray.ptype().to_unsigned());
-    let packed = match_each_unsigned_integer_ptype!(parray.ptype(), |P| {
+    match_each_unsigned_integer_ptype!(parray.ptype(), |P| {
         bitpack_primitive(parray.as_slice::<P>(), bit_width).into_byte_buffer()
-    });
-    Ok(packed)
+    })
 }
 
 /// Bitpack a slice of primitives down to the given width.