Mooooaarrrrr generics

Signed-off-by: Nicholas Gates <[email protected]>

Author: gatesn

.github/release-drafter.yml

@@ -15,6 +15,9 @@ categories:
     collapse-after: 8
     labels:
       - "fix"
+  - title: "📖 Documentation"
+    labels:
+      - "documentation"
   - title: "🧰 Maintenance"
     labels:
       - "chore"

.github/workflows/labels.yml

@@ -28,6 +28,7 @@ jobs:
         run: |
           REQUIRED_LABELS=(
             "chore"
+            "documentation"
             "feature"
             "fix"
             "performance"

vortex-compute/src/filter/bitbuffer.rs

@@ -7,7 +7,7 @@ use vortex_buffer::{
 };
 use vortex_mask::Mask;
 
-use crate::filter::{Filter, MaskIndices};
+use crate::filter::Filter;
 
 impl Filter<Mask> for &BitBuffer {
     type Output = BitBuffer;
@@ -49,22 +49,6 @@ impl Filter<Mask> for &mut BitBufferMut {
     }
 }
 
-impl Filter<MaskIndices<'_>> for &BitBuffer {
-    type Output = BitBuffer;
-
-    fn filter(self, indices: &MaskIndices) -> BitBuffer {
-        filter_indices(self.inner().as_ref(), self.offset(), indices).freeze()
-    }
-}
-
-impl Filter<MaskIndices<'_>> for &mut BitBufferMut {
-    type Output = ();
-
-    fn filter(self, indices: &MaskIndices) {
-        *self = filter_indices(self.inner().as_ref(), self.offset(), indices)
-    }
-}
-
 fn filter_indices(bools: &[u8], bit_offset: usize, indices: &[usize]) -> BitBufferMut {
     // FIXME(ngates): this is slower than it could be!
     BitBufferMut::collect_bool(indices.len(), |idx| {

vortex-compute/src/filter/buffer.rs

@@ -4,7 +4,10 @@
 use vortex_buffer::{BitView, Buffer, BufferMut};
 use vortex_mask::{Mask, MaskIter};
 
-use crate::filter::{Filter, MaskIndices};
+use crate::filter::Filter;
+
+// This is modeled after the constant with the equivalent name in arrow-rs.
+const FILTER_SLICES_SELECTIVITY_THRESHOLD: f64 = 0.8;
 
 impl<M, T: Copy> Filter<M> for Buffer<T>
 where
@@ -26,9 +29,6 @@ where
     }
 }
 
-// This is modeled after the constant with the equivalent name in arrow-rs.
-const FILTER_SLICES_SELECTIVITY_THRESHOLD: f64 = 0.8;
-
 impl<T: Copy> Filter<Mask> for &Buffer<T> {
     type Output = Buffer<T>;
 
@@ -52,14 +52,6 @@ impl<T: Copy> Filter<Mask> for &Buffer<T> {
     }
 }
 
-impl<T: Copy> Filter<MaskIndices<'_>> for &Buffer<T> {
-    type Output = Buffer<T>;
-
-    fn filter(self, indices: &MaskIndices) -> Buffer<T> {
-        filter_indices(self, indices)
-    }
-}
-
 impl<const NB: usize, T: Copy> Filter<BitView<'_, NB>> for &Buffer<T> {
     type Output = Buffer<T>;
 

vortex-compute/src/filter/mask.rs

@@ -5,7 +5,7 @@ use vortex_buffer::BitView;
 use vortex_error::VortexExpect;
 use vortex_mask::{Mask, MaskMut};
 
-use crate::filter::{Filter, MaskIndices};
+use crate::filter::Filter;
 
 impl Filter<Mask> for &Mask {
     type Output = Mask;
@@ -30,18 +30,6 @@ impl Filter<Mask> for &Mask {
     }
 }
 
-impl Filter<MaskIndices<'_>> for &Mask {
-    type Output = Mask;
-
-    fn filter(self, indices: &MaskIndices<'_>) -> Mask {
-        match self {
-            Mask::AllTrue(_) => Mask::AllTrue(indices.len()),
-            Mask::AllFalse(_) => Mask::AllFalse(indices.len()),
-            Mask::Values(mask_values) => Mask::from(mask_values.bit_buffer().filter(indices)),
-        }
-    }
-}
-
 impl<const NB: usize> Filter<BitView<'_, NB>> for &Mask {
     type Output = Mask;
 
@@ -70,16 +58,6 @@ impl Filter<Mask> for &mut MaskMut {
     }
 }
 
-impl Filter<MaskIndices<'_>> for &mut MaskMut {
-    type Output = ();
-
-    fn filter(self, indices: &MaskIndices<'_>) -> Self::Output {
-        // TODO(aduffy): Filter in-place
-        let filtered = self.clone().freeze().filter(indices).into_mut();
-        *self = filtered;
-    }
-}
-
 impl<const NB: usize> Filter<BitView<'_, NB>> for &mut MaskMut {
     type Output = ();
 

vortex-compute/src/filter/mod.rs

@@ -3,8 +3,6 @@
 
 //! Filter function.
 
-use std::ops::Deref;
-
 mod bitbuffer;
 mod buffer;
 mod mask;
@@ -25,35 +23,3 @@ pub trait Filter<By: ?Sized> {
     /// If the length of the mask does not equal the length of the value being filtered.
     fn filter(self, selection: &By) -> Self::Output;
 }
-
-/// A view over a set of strictly sorted indices from a bit mask.
-///
-/// Unlike other indices, `MaskIndices` are always strict-sorted, meaning they are
-/// always unique and monotonic.
-///
-/// You can treat a `MaskIndices` just like a `&[usize]` by iterating or indexing
-/// into it just like you would a slice.
-pub struct MaskIndices<'a>(&'a [usize]);
-
-impl<'a> MaskIndices<'a> {
-    /// Create new indices from a slice of strict-sorted index values.
-    ///
-    /// # Safety
-    ///
-    /// The caller must ensure that the indices are strict-sorted, i.e. that they
-    /// are monotonic and unique.
-    ///
-    /// Users of the `Indices` type assume this and failure to uphold this guarantee
-    /// can result in UB downstream.
-    pub unsafe fn new_unchecked(indices: &'a [usize]) -> Self {
-        Self(indices)
-    }
-}
-
-impl Deref for MaskIndices<'_> {
-    type Target = [usize];
-
-    fn deref(&self) -> &Self::Target {
-        self.0
-    }
-}

vortex-compute/src/filter/slice_mut.rs

@@ -12,18 +12,7 @@ use std::ptr;
 use vortex_buffer::BitView;
 use vortex_mask::Mask;
 
-use crate::filter::{Filter, MaskIndices};
-
-impl<T: Copy> Filter<MaskIndices<'_>> for &mut [T] {
-    type Output = Self;
-
-    fn filter(self, indices: &MaskIndices) -> Self::Output {
-        for (write_index, &read_index) in indices.iter().enumerate() {
-            self[write_index] = self[read_index];
-        }
-        &mut self[..indices.len()]
-    }
-}
+use crate::filter::Filter;
 
 impl<T: Copy> Filter<Mask> for &mut [T] {
     type Output = Self;

vortex-compute/src/filter/vector/binaryview.rs

@@ -42,175 +42,3 @@ where
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use vortex_mask::Mask;
-    use vortex_vector::binaryview::StringVectorMut;
-    use vortex_vector::{VectorMutOps, VectorOps};
-
-    use super::*;
-    use crate::filter::MaskIndices;
-
-    #[test]
-    fn test_filter_binary_view_vector_with_mask() {
-        let mut vec = StringVectorMut::with_capacity(5);
-        vec.append_values("hello", 1);
-        vec.append_values("world", 1);
-        vec.append_values("foo", 1);
-        vec.append_values("bar", 1);
-        vec.append_values("baz", 1);
-        let vec = vec.freeze();
-
-        let mask = Mask::from_iter([true, false, true, false, true]);
-
-        let filtered = vec.filter(&mask);
-
-        assert_eq!(filtered.len(), 3);
-        assert_eq!(filtered.validity().true_count(), 3);
-        assert_eq!(filtered.get_ref(0), Some("hello"));
-        assert_eq!(filtered.get_ref(1), Some("foo"));
-        assert_eq!(filtered.get_ref(2), Some("baz"));
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_with_mask_indices() {
-        let mut vec = StringVectorMut::with_capacity(5);
-        vec.append_values("hello", 1);
-        vec.append_values("world", 1);
-        vec.append_values("foo", 1);
-        vec.append_values("bar", 1);
-        vec.append_values("baz", 1);
-        let vec = vec.freeze();
-
-        let indices = unsafe { MaskIndices::new_unchecked(&[0, 2, 4]) };
-
-        let filtered = vec.filter(&indices);
-
-        assert_eq!(filtered.len(), 3);
-        assert_eq!(filtered.validity().true_count(), 3);
-        assert_eq!(filtered.get_ref(0), Some("hello"));
-        assert_eq!(filtered.get_ref(1), Some("foo"));
-        assert_eq!(filtered.get_ref(2), Some("baz"));
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_with_nulls() {
-        let mut vec = StringVectorMut::with_capacity(5);
-        vec.append_values("hello", 1);
-        vec.append_nulls(1);
-        vec.append_values("foo", 1);
-        vec.append_values("bar", 1);
-        vec.append_nulls(1);
-        let vec = vec.freeze();
-
-        let mask = Mask::from_iter([true, true, false, true, false]);
-
-        let filtered = vec.filter(&mask);
-
-        assert_eq!(filtered.len(), 3);
-        assert_eq!(filtered.validity().true_count(), 2);
-        assert_eq!(filtered.get_ref(0), Some("hello"));
-        assert_eq!(filtered.get_ref(1), None);
-        assert_eq!(filtered.get_ref(2), Some("bar"));
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_all_true() {
-        let mut vec = StringVectorMut::with_capacity(3);
-        vec.append_values("hello", 1);
-        vec.append_values("world", 1);
-        vec.append_values("foo", 1);
-        let vec = vec.freeze();
-
-        let mask = Mask::new_true(3);
-
-        let filtered = vec.filter(&mask);
-
-        assert_eq!(filtered.len(), 3);
-        assert_eq!(filtered.get_ref(0), Some("hello"));
-        assert_eq!(filtered.get_ref(1), Some("world"));
-        assert_eq!(filtered.get_ref(2), Some("foo"));
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_all_false() {
-        let mut vec = StringVectorMut::with_capacity(3);
-        vec.append_values("hello", 1);
-        vec.append_values("world", 1);
-        vec.append_values("foo", 1);
-        let vec = vec.freeze();
-
-        let mask = Mask::new_false(3);
-
-        let filtered = vec.filter(&mask);
-
-        assert_eq!(filtered.len(), 0);
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_mut_with_mask() {
-        let mut vec = StringVectorMut::with_capacity(5);
-        vec.append_values("hello", 1);
-        vec.append_values("world", 1);
-        vec.append_values("foo", 1);
-        vec.append_values("bar", 1);
-        vec.append_values("baz", 1);
-
-        let mask = Mask::from_iter([true, false, true, false, true]);
-
-        vec.filter(&mask);
-
-        assert_eq!(vec.len(), 3);
-        let frozen = vec.freeze();
-        assert_eq!(frozen.len(), 3);
-        assert_eq!(frozen.validity().true_count(), 3);
-        assert_eq!(frozen.get_ref(0), Some("hello"));
-        assert_eq!(frozen.get_ref(1), Some("foo"));
-        assert_eq!(frozen.get_ref(2), Some("baz"));
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_mut_with_mask_indices() {
-        let mut vec = StringVectorMut::with_capacity(5);
-        vec.append_values("hello", 1);
-        vec.append_values("world", 1);
-        vec.append_values("foo", 1);
-        vec.append_values("bar", 1);
-        vec.append_values("baz", 1);
-
-        let indices = unsafe { MaskIndices::new_unchecked(&[0, 2, 4]) };
-
-        vec.filter(&indices);
-
-        assert_eq!(vec.len(), 3);
-        let frozen = vec.freeze();
-        assert_eq!(frozen.len(), 3);
-        assert_eq!(frozen.validity().true_count(), 3);
-        assert_eq!(frozen.get_ref(0), Some("hello"));
-        assert_eq!(frozen.get_ref(1), Some("foo"));
-        assert_eq!(frozen.get_ref(2), Some("baz"));
-    }
-
-    #[test]
-    fn test_filter_binary_view_vector_mut_with_nulls() {
-        let mut vec = StringVectorMut::with_capacity(5);
-        vec.append_values("hello", 1);
-        vec.append_nulls(1);
-        vec.append_values("foo", 1);
-        vec.append_values("bar", 1);
-        vec.append_nulls(1);
-
-        let mask = Mask::from_iter([true, true, false, true, false]);
-
-        vec.filter(&mask);
-
-        assert_eq!(vec.len(), 3);
-        let frozen = vec.freeze();
-        assert_eq!(frozen.len(), 3);
-        assert_eq!(frozen.validity().true_count(), 2);
-        assert_eq!(frozen.get_ref(0), Some("hello"));
-        assert_eq!(frozen.get_ref(1), None);
-        assert_eq!(frozen.get_ref(2), Some("bar"));
-    }
-}