fixes

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

Author: robert3005

vortex-array/src/arrays/varbin/builder.rs

@@ -630,10 +630,6 @@ impl<O: OffsetBuilderPType> ArrayBuilder for VarBinBuilder<O> {
         self.validity.reserve(additional);
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        self.replace_validity(validity)
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_varbin().into_array()
     }
@@ -949,7 +945,6 @@ mod tests {
                 builder.append_scalar(&Scalar::utf8("hello", Nullable))?;
                 builder.append_null();
                 assert_eq!(builder.len(), 3);
-                builder.set_validity(validity.clone());
                 Ok(())
             })?;
             assert_eq!(result.validity()?.execute_mask(3, &mut ctx)?, validity);

vortex-array/src/builders/bool.rs

@@ -7,7 +7,6 @@ use std::mem;
 use vortex_buffer::BitBufferMut;
 use vortex_error::VortexResult;
 use vortex_error::vortex_ensure;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::ExecutionCtx;
@@ -127,10 +126,6 @@ impl ArrayBuilder for BoolBuilder {
         self.nulls.reserve_exact(additional);
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        self.nulls = LazyBitBufferBuilder::from_validity_mask(validity);
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_bool().into_array()
     }

vortex-array/src/builders/child.rs

@@ -1,22 +1,17 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 use vortex_error::vortex_ensure;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::ExecutionCtx;
 use crate::IntoArray;
 use crate::arrays::ChunkedArray;
-use crate::arrays::MaskedArray;
 use crate::builders::ArrayBuilder;
 use crate::builders::builder_with_capacity;
 use crate::dtype::DType;
-use crate::dtype::Nullability;
 use crate::scalar::Scalar;
-use crate::validity::Validity;
 
 /// Accumulates the child of a nested [`ArrayBuilder`] without canonicalizing appended arrays.
 ///
@@ -121,42 +116,6 @@ impl ChildBuilder {
         self.pending.reserve_exact(additional)
     }
 
-    /// Overrides the validity of every value appended so far.
-    ///
-    /// # Safety
-    ///
-    /// `validity` must have the same length as [`self.len()`](Self::len).
-    ///
-    /// # Panics
-    ///
-    /// Panics if a chunk that was kept in its original encoding contains nulls, since replacing
-    /// the validity of such a chunk would require decoding it.
-    pub unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        if !self.dtype.is_nullable() {
-            return;
-        }
-
-        if self.chunks.is_empty() {
-            // Fast path: every value lives in the scalar builder, which owns its null buffer.
-            unsafe { self.pending.set_validity_unchecked(validity) };
-            return;
-        }
-
-        // The chunks carry their own validity, so the override has to be pushed into each of them.
-        self.flush_pending();
-        let mut offset = 0;
-        for chunk in &mut self.chunks {
-            let end = offset + chunk.len();
-            *chunk = MaskedArray::try_new(
-                chunk.clone(),
-                Validity::from_mask(validity.slice(offset..end), Nullability::Nullable),
-            )
-            .vortex_expect("cannot override the validity of a child chunk that contains nulls")
-            .into_array();
-            offset = end;
-        }
-    }
-
     /// Finishes the child, combining the accumulated chunks into a [`ChunkedArray`] when there is
     /// more than one of them.
     pub fn finish(&mut self) -> ArrayRef {
@@ -172,9 +131,7 @@ impl ChildBuilder {
             return chunks.remove(0);
         }
 
-        ChunkedArray::try_new(chunks, self.dtype.clone())
-            .vortex_expect("every child chunk has the child dtype")
-            .into_array()
+        unsafe { ChunkedArray::new_unchecked(chunks, self.dtype.clone()) }.into_array()
     }
 
     /// Moves whatever the scalar builder holds into `chunks`, keeping the chunks in logical order.
@@ -192,9 +149,7 @@ impl ChildBuilder {
 mod tests {
     use rstest::rstest;
     use vortex_buffer::buffer;
-    use vortex_error::VortexExpect;
     use vortex_error::VortexResult;
-    use vortex_mask::Mask;
 
     use super::ChildBuilder;
     use crate::ArrayRef;
@@ -205,7 +160,6 @@ mod tests {
     use crate::arrays::ChunkedArray;
     use crate::arrays::Constant;
     use crate::arrays::ConstantArray;
-    use crate::arrays::Masked;
     use crate::arrays::Primitive;
     use crate::arrays::PrimitiveArray;
     use crate::arrays::chunked::ChunkedArrayExt;
@@ -380,104 +334,6 @@ mod tests {
         Ok(())
     }
 
-    /// Overriding the validity once chunks exist has to push the override into each chunk, sliced
-    /// to that chunk's own range.
-    #[test]
-    fn test_set_validity_pushes_the_override_into_every_chunk() -> VortexResult<()> {
-        let mut ctx = array_session().create_execution_ctx();
-        let dtype = DType::Primitive(I32, Nullable);
-        let mut builder = ChildBuilder::with_capacity(&dtype, 0);
-
-        builder.append_array(&nullable_constant(1, CHUNK_LEN), &mut ctx)?;
-        builder.append_array(&nullable_constant(2, CHUNK_LEN), &mut ctx)?;
-
-        // Straddle the chunk boundary, so an override sliced wrongly cannot pass.
-        let invalid = [CHUNK_LEN - 1, CHUNK_LEN];
-        let validity = Mask::from_iter((0..2 * CHUNK_LEN).map(|i| !invalid.contains(&i)));
-        unsafe { builder.set_validity_unchecked(validity) };
-
-        let child = builder.finish();
-        let chunked = child.as_::<Chunked>();
-        assert_eq!(chunked.nchunks(), 2);
-        // The override was layered over the chunks rather than decoding them.
-        assert!(chunked.iter_chunks().all(|chunk| chunk.is::<Masked>()));
-
-        let expected = PrimitiveArray::from_option_iter(
-            (0..2 * CHUNK_LEN)
-                .map(|i| (!invalid.contains(&i)).then_some(if i < CHUNK_LEN { 1i32 } else { 2 })),
-        )
-        .into_array();
-        assert_arrays_eq!(&child, &expected, &mut ctx);
-
-        Ok(())
-    }
-
-    /// Values still sitting in the scalar builder are part of the override too.
-    #[test]
-    fn test_set_validity_covers_pending_scalars() -> VortexResult<()> {
-        let mut ctx = array_session().create_execution_ctx();
-        let dtype = DType::Primitive(I32, Nullable);
-        let mut builder = ChildBuilder::with_capacity(&dtype, 0);
-
-        builder.append_array(&nullable_constant(1, CHUNK_LEN), &mut ctx)?;
-        builder.append_scalar(&Scalar::primitive(2i32, Nullable))?;
-
-        let mut validity = vec![true; CHUNK_LEN + 1];
-        validity[CHUNK_LEN] = false;
-        unsafe { builder.set_validity_unchecked(Mask::from_iter(validity)) };
-
-        let child = builder.finish();
-        let expected = PrimitiveArray::from_option_iter(
-            std::iter::repeat_n(Some(1i32), CHUNK_LEN).chain([None]),
-        )
-        .into_array();
-        assert_arrays_eq!(&child, &expected, &mut ctx);
-
-        Ok(())
-    }
-
-    /// A non-nullable child cannot carry nulls, so the override is dropped and the chunks are left
-    /// exactly as they were appended.
-    #[test]
-    fn test_set_validity_is_a_noop_for_a_non_nullable_child() -> VortexResult<()> {
-        let mut ctx = array_session().create_execution_ctx();
-        let mut builder = ChildBuilder::with_capacity(&DType::from(I32), 0);
-
-        builder.append_array(&constant(1, CHUNK_LEN), &mut ctx)?;
-        builder.append_array(&constant(2, CHUNK_LEN), &mut ctx)?;
-        unsafe { builder.set_validity_unchecked(Mask::new_false(2 * CHUNK_LEN)) };
-
-        let child = builder.finish();
-        let chunked = child.as_::<Chunked>();
-        assert!(chunked.iter_chunks().all(|chunk| chunk.is::<Constant>()));
-
-        let expected = ChunkedArray::try_new(
-            vec![constant(1, CHUNK_LEN), constant(2, CHUNK_LEN)],
-            DType::from(I32),
-        )?
-        .into_array();
-        assert_arrays_eq!(&child, &expected, &mut ctx);
-
-        Ok(())
-    }
-
-    /// Replacing the validity of a chunk that already contains nulls would mean decoding it, which
-    /// is exactly what the chunk exists to avoid.
-    #[test]
-    #[should_panic(expected = "cannot override the validity of a child chunk that contains nulls")]
-    fn test_set_validity_rejects_a_chunk_that_contains_nulls() {
-        let mut ctx = array_session().create_execution_ctx();
-        let dtype = DType::Primitive(I32, Nullable);
-        let mut builder = ChildBuilder::with_capacity(&dtype, 0);
-
-        let with_nulls = ConstantArray::new(Scalar::null(dtype), CHUNK_LEN).into_array();
-        builder
-            .append_array(&with_nulls, &mut ctx)
-            .vortex_expect("append");
-
-        unsafe { builder.set_validity_unchecked(Mask::new_true(CHUNK_LEN)) };
-    }
-
     #[test]
     fn test_finish_resets_the_builder() -> VortexResult<()> {
         let mut ctx = array_session().create_execution_ctx();

vortex-array/src/builders/decimal.rs

@@ -9,7 +9,6 @@ use vortex_error::VortexResult;
 use vortex_error::vortex_ensure;
 use vortex_error::vortex_err;
 use vortex_error::vortex_panic;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::ExecutionCtx;
@@ -219,10 +218,6 @@ impl ArrayBuilder for DecimalBuilder {
         self.nulls.reserve_exact(additional);
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        self.nulls = LazyBitBufferBuilder::from_validity_mask(validity);
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_decimal().into_array()
     }

vortex-array/src/builders/extension.rs

@@ -5,7 +5,6 @@ use std::any::Any;
 
 use vortex_error::VortexResult;
 use vortex_error::vortex_ensure;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::ExecutionCtx;
@@ -114,10 +113,6 @@ impl ArrayBuilder for ExtensionBuilder {
         self.storage.reserve_exact(capacity)
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        unsafe { self.storage.set_validity_unchecked(validity) };
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_extension().into_array()
     }

vortex-array/src/builders/fixed_size_list.rs

@@ -9,7 +9,6 @@ use vortex_error::VortexResult;
 use vortex_error::vortex_bail;
 use vortex_error::vortex_ensure;
 use vortex_error::vortex_panic;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::ExecutionCtx;
@@ -260,10 +259,6 @@ impl ArrayBuilder for FixedSizeListBuilder {
         self.nulls.reserve_exact(additional);
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        self.nulls.set_validity(validity);
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_fixed_size_list().into_array()
     }

vortex-array/src/builders/lazy_null_builder.rs

@@ -31,42 +31,6 @@ impl LazyBitBufferBuilder {
         }
     }
 
-    /// Creates a builder pre-populated from a validity mask, taking ownership of the mask's buffer
-    /// instead of copying it where possible.
-    ///
-    /// This is the counterpart to [`append_validity_mask`](Self::append_validity_mask) for callers
-    /// that want to *replace* the builder's contents with the mask rather than extend them: because
-    /// we own the mask, we can move its buffer in instead of copying it.
-    pub fn from_validity_mask(validity_mask: Mask) -> Self {
-        match validity_mask {
-            // An unmaterialized builder already represents `len` non-null values, so an all-valid
-            // mask stays lazy.
-            Mask::AllTrue(len) => Self {
-                inner: None,
-                len,
-                capacity: len,
-            },
-            Mask::AllFalse(len) => Self::from_buffer(BitBufferMut::new_unset(len)),
-            // Take ownership of the underlying buffer; `into_bit_buffer` and `try_into_mut` only
-            // copy when the buffer is shared, otherwise this is a move.
-            values @ Mask::Values(_) => Self::from_buffer(
-                values
-                    .into_bit_buffer()
-                    .try_into_mut()
-                    .unwrap_or_else(|buffer| BitBufferMut::copy_from(&buffer)),
-            ),
-        }
-    }
-
-    /// Creates a builder backed by an already-materialized buffer.
-    fn from_buffer(inner: BitBufferMut) -> Self {
-        Self {
-            inner: Some(inner),
-            len: 0,
-            capacity: 0,
-        }
-    }
-
     /// Appends `n` non-null values to the builder.
     #[inline]
     pub fn append_n_non_nulls(&mut self, n: usize) {

vortex-array/src/builders/list.rs

@@ -10,7 +10,6 @@ use vortex_error::VortexResult;
 use vortex_error::vortex_bail;
 use vortex_error::vortex_ensure;
 use vortex_error::vortex_panic;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::Canonical;
@@ -377,10 +376,6 @@ impl<O: OffsetBuilderPType> ArrayBuilder for ListBuilder<O> {
         self.nulls.reserve_exact(additional);
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        self.nulls.set_validity(validity);
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_list().into_array()
     }

vortex-array/src/builders/listview.rs

@@ -16,7 +16,6 @@ use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 use vortex_error::vortex_ensure;
 use vortex_error::vortex_panic;
-use vortex_mask::Mask;
 
 use crate::ArrayRef;
 use crate::Canonical;
@@ -416,10 +415,6 @@ impl<O: OffsetBuilderPType, S: OffsetBuilderPType> ArrayBuilder for ListViewBuil
         self.nulls.reserve_exact(capacity);
     }
 
-    unsafe fn set_validity_unchecked(&mut self, validity: Mask) {
-        self.nulls.set_validity(validity);
-    }
-
     fn finish(&mut self) -> ArrayRef {
         self.finish_into_listview().into_array()
     }