fix: BitView slice filter

I wrote the unit test, it failed, stepped through it in debugger,
noticed that span was always zero. We always clear the lowest bit and
then check for trailing_ones, so we never copied anything.

Fixes the logic to actually copy runs of values. Test passes now.

Signed-off-by: Andrew Duffy <[email protected]>

Author: a10y

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

@@ -203,12 +203,15 @@ impl<BP: PhysicalPType<Physical: BitPacking>> Kernel for AlignedBitPackedKernel<
 
 #[cfg(test)]
 mod tests {
+    use itertools::Itertools;
+    use vortex_array::IntoArray;
     use vortex_array::arrays::PrimitiveArray;
-    use vortex_dtype::PTypeDowncast;
+    use vortex_dtype::{PTypeDowncast, PTypeDowncastExt};
     use vortex_mask::Mask;
     use vortex_vector::VectorOps;
 
     use crate::BitPackedArray;
+    use crate::bitpack_compress::bitpack_encode;
 
     #[test]
     fn test_bitpack_pipeline_basic() {
@@ -408,4 +411,24 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_pipeline() {
+        let array = PrimitiveArray::from_iter(0u64..2048u64);
+        let packed = bitpack_encode(&array, 12, None).unwrap().into_array();
+
+        // Only select odd numbered elements
+        let select_indices = (0..2048).filter(|i| i % 2 == 1).collect_vec();
+        let selection = Mask::from_indices(2048, select_indices);
+
+        let result = packed.execute_with_selection(&selection).unwrap();
+        assert_eq!(result.len(), 1024);
+
+        let result = result.into_primitive().downcast::<u64>();
+
+        let slice = result.as_ref();
+        for i in 0..1024 {
+            assert_eq!(slice[i], (2 * i + 1) as u64);
+        }
+    }
 }

vortex-compute/src/filter/slice_mut.rs

@@ -108,17 +108,41 @@ impl<'a, const NB: usize, T: Copy> Filter<BitView<'a, NB>> for &mut [T] {
                     }
                 }
                 _ => {
+                    // Iterate the bits in a word, attempting to copy contiguous runs of values.
+                    let mut read_pos = 0;
+                    let mut write_pos = 0;
                     while word != 0 {
-                        let bit_pos = word.trailing_zeros();
-                        word &= word - 1; // Clear the bit at `bit_pos`
-                        let span = word.trailing_ones();
-                        word >>= span;
+                        let tz = word.trailing_zeros();
+                        if tz > 0 {
+                            // shift off the trailing zeros since they are unselected.
+                            // this advances the read head, but not the write head.
+                            read_pos += tz;
+                            word >>= tz;
+                            continue;
+                        }
+
+                        // copy the next several values to our out pointer.
+                        let extent = word.trailing_ones();
                         unsafe {
-                            ptr::copy(read_ptr.add(bit_pos as usize), write_ptr, span as usize);
-                            write_ptr = write_ptr.add(span as usize);
+                            ptr::copy(
+                                read_ptr.add(read_pos as usize),
+                                write_ptr.add(write_pos as usize),
+                                extent as usize,
+                            );
                         }
+                        // Advance the reader and writer by the number of values
+                        // we just copied.
+                        read_pos += extent;
+                        write_pos += extent;
+
+                        // shift off the low bits of the word so we can copy the next run.
+                        word >>= extent;
                     }
-                    unsafe { read_ptr = read_ptr.add(usize::BITS as usize) };
+
+                    unsafe {
+                        read_ptr = read_ptr.add(usize::BITS as usize);
+                        write_ptr = write_ptr.add(write_pos as usize);
+                    };
                 }
             }
         }