VarBinVector

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

Author: gatesn

vortex-compute/src/mask/mod.rs

@@ -8,8 +8,8 @@ use std::ops::BitAnd;
 use vortex_dtype::NativePType;
 use vortex_mask::Mask;
 use vortex_vector::{
-    BoolVector, NullVector, PrimitiveVector, StructVector, Vector, match_each_pvector,
-    match_each_vector,
+    BoolVector, NullVector, PVector, PrimitiveVector, StructVector, VarBinType, VarBinVector,
+    Vector, match_each_pvector, match_each_vector,
 };
 
 /// Trait for masking the validity of an array or vector.
@@ -37,7 +37,8 @@ impl MaskValidity for NullVector {
 impl MaskValidity for BoolVector {
     fn mask_validity(self, mask: &Mask) -> Self {
         let (bits, validity) = self.into_parts();
-        Self::new(bits, validity.bitand(mask))
+        // SAFETY: we are preserving the original bits buffer and only modifying the validity.
+        unsafe { Self::new_unchecked(bits, validity.bitand(mask)) }
     }
 }
 
@@ -47,16 +48,26 @@ impl MaskValidity for PrimitiveVector {
     }
 }
 
-impl<T: NativePType> MaskValidity for vortex_vector::PVector<T> {
+impl<T: NativePType> MaskValidity for PVector<T> {
     fn mask_validity(self, mask: &Mask) -> Self {
         let (data, validity) = self.into_parts();
-        Self::new(data, validity.bitand(mask))
+        // SAFETY: we are preserving the original data buffer and only modifying the validity.
+        unsafe { Self::new_unchecked(data, validity.bitand(mask)) }
+    }
+}
+
+impl<T: VarBinType> MaskValidity for VarBinVector<T> {
+    fn mask_validity(self, mask: &Mask) -> Self {
+        let (views, buffers, validity) = self.into_parts();
+        // SAFETY: we are preserving the original views and buffers, only modifying the validity.
+        unsafe { Self::new_unchecked(views, buffers, validity.bitand(mask)) }
     }
 }
 
 impl MaskValidity for StructVector {
     fn mask_validity(self, mask: &Mask) -> Self {
         let (fields, validity) = self.into_parts();
-        StructVector::new(fields, validity.bitand(mask))
+        // SAFETY: we are preserving the original fields and only modifying the validity.
+        unsafe { StructVector::new_unchecked(fields, validity.bitand(mask)) }
     }
 }

vortex-vector/src/lib.rs

@@ -6,7 +6,6 @@
 //! Immutable and mutable decompressed (canonical) vectors for Vortex.
 
 #![deny(missing_docs)]
-#![deny(clippy::missing_docs_in_private_items)]
 #![deny(clippy::missing_errors_doc)]
 #![deny(clippy::missing_panics_doc)]
 #![deny(clippy::missing_safety_doc)]

vortex-vector/src/varbin/mod.rs

@@ -29,7 +29,6 @@ impl VarBinTypeDowncast for Vector {
         if let Vector::Binary(v) = self {
             return v;
         }
-
         vortex_panic!("Expected BinaryVector, got {self:?}");
     }
 

vortex-vector/src/varbin/types.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+//! Variable-length binary types and related traits.
+
 use std::fmt::Debug;
 
 use crate::{VarBinVector, VarBinVectorMut, Vector, VectorMut};
@@ -81,8 +83,11 @@ pub trait VarBinTypeUpcast {
     fn from_string(input: Self::Input<StringType>) -> Self;
 }
 
+/// Private module to seal the [`VarBinType`] trait.
 mod private {
+    /// Sealed trait to prevent external implementations of [`VarBinType`].
     pub trait Sealed {}
+
     impl Sealed for super::StringType {}
     impl Sealed for super::BinaryType {}
 }

vortex-vector/src/varbin/vector.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+//! Variable-length binary vector implementation.
+
 use std::sync::Arc;
 
 use vortex_buffer::{Buffer, ByteBuffer};
@@ -14,9 +16,13 @@ use crate::varbin::view::BinaryView;
 /// A variable-length binary vector.
 #[derive(Debug, Clone)]
 pub struct VarBinVector<T: VarBinType> {
+    /// Views into the binary data.
     views: Buffer<BinaryView>,
-    validity: Mask,
+    /// Buffers holding the referenced binary data.
     buffers: Arc<Box<[ByteBuffer]>>,
+    /// Validity mask for the vector.
+    validity: Mask,
+    /// Marker trait for the [`VarBinType`].
     _marker: std::marker::PhantomData<T>,
 }
 
@@ -33,16 +39,21 @@ impl<T: VarBinType> VarBinVector<T> {
     /// - The `views` buffer correctly references the data in the `buffers`.
     pub unsafe fn new_unchecked(
         views: Buffer<BinaryView>,
-        validity: Mask,
         buffers: Arc<Box<[ByteBuffer]>>,
+        validity: Mask,
     ) -> Self {
         Self {
             views,
-            validity,
             buffers,
+            validity,
             _marker: std::marker::PhantomData,
         }
     }
+
+    /// Decomposes the vector into its constituent parts.
+    pub fn into_parts(self) -> (Buffer<BinaryView>, Arc<Box<[ByteBuffer]>>, Mask) {
+        (self.views, self.buffers, self.validity)
+    }
 }
 
 impl<T: VarBinType> VectorOps for VarBinVector<T> {

vortex-vector/src/varbin/vector_mut.rs

@@ -1,7 +1,9 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-use vortex_buffer::{BufferMut, ByteBuffer};
+//! Mutable variable-length binary vector.
+
+use vortex_buffer::{BufferMut, ByteBuffer, ByteBufferMut};
 use vortex_mask::MaskMut;
 
 use crate::VectorMutOps;
@@ -10,33 +12,23 @@ use crate::varbin::vector::VarBinVector;
 use crate::varbin::view::BinaryView;
 
 /// Mutable variable-length binary vector.
+#[allow(dead_code)] // FIXME(ngates): remove after implementing the methods
 #[derive(Clone, Debug)]
 pub struct VarBinVectorMut<T: VarBinType> {
+    /// Views into the binary data.
     views: BufferMut<BinaryView>,
+    /// Validity mask for the vector.
     validity: MaskMut,
 
+    /// The completed buffers holding referenced binary data.
     buffers: Vec<ByteBuffer>,
-    open_buffer: Option<ByteBuffer>,
+    /// The current buffer being appended to, if any.
+    open_buffer: Option<ByteBufferMut>,
 
+    /// Marker trait for the [`VarBinType`].
     _marker: std::marker::PhantomData<T>,
 }
 
-impl<T: VarBinType> VarBinVectorMut<T> {
-    pub(super) fn new(
-        views: BufferMut<BinaryView>,
-        validity: MaskMut,
-        buffers: Vec<ByteBuffer>,
-    ) -> Self {
-        Self {
-            views,
-            validity,
-            buffers,
-            open_buffer: None,
-            _marker: std::marker::PhantomData,
-        }
-    }
-}
-
 impl<T: VarBinType> VectorMutOps for VarBinVectorMut<T> {
     type Immutable = VarBinVector<T>;
 
@@ -52,7 +44,7 @@ impl<T: VarBinType> VectorMutOps for VarBinVectorMut<T> {
         self.views.reserve(additional);
     }
 
-    fn extend_from_vector(&mut self, other: &Self::Immutable) {
+    fn extend_from_vector(&mut self, _other: &Self::Immutable) {
         todo!()
     }
 
@@ -65,11 +57,11 @@ impl<T: VarBinType> VectorMutOps for VarBinVectorMut<T> {
         todo!()
     }
 
-    fn split_off(&mut self, at: usize) -> Self {
+    fn split_off(&mut self, _at: usize) -> Self {
         todo!()
     }
 
-    fn unsplit(&mut self, other: Self) {
+    fn unsplit(&mut self, _other: Self) {
         todo!()
     }
 }