VX-45 Validate NULL views in VarBinView

vortex-array/src/arrays/varbinview/array.rs

@@ -323,8 +323,11 @@ impl VarBinViewData {
     where
         F: Fn(&[u8]) -> bool,
     {
-        let validate_view = |idx: usize, view: &BinaryView| -> VortexResult<()> {
+        let validate_view = |idx: usize, view: &BinaryView, valid: bool| -> VortexResult<()> {
             if view.is_inlined() {
+                if !valid {
+                    return Ok(());
+                }
                 // Validate the inline bytestring
                 let bytes = &view.as_inlined().data[..view.len() as usize];
                 vortex_ensure!(
@@ -354,6 +357,10 @@ impl VarBinViewData {
                     buf.len(),
                 );
 
+                if !valid {
+                    return Ok(());
+                }
+
                 // Make sure the prefix data matches the buffer data.
                 let bytes = &buf[start_offset..end_offset];
                 vortex_ensure!(
@@ -378,17 +385,18 @@ impl VarBinViewData {
                 let mut ctx = legacy_session().create_execution_ctx();
                 let mask = validity.execute_mask(views.len(), &mut ctx)?;
                 for ((idx, view), valid) in views.iter().enumerate().zip(mask.iter()) {
-                    if valid {
-                        validate_view(idx, view)?;
-                    }
+                    validate_view(idx, view, valid)?;
+                }
+            }
+            Validity::AllInvalid => {
+                for (idx, view) in views.iter().enumerate() {
+                    validate_view(idx, view, false)?;
                 }
             }
-            // Every entry is null, so there is nothing to validate.
-            Validity::AllInvalid => {}
             // No nulls: validate every view.
             Validity::NonNullable | Validity::AllValid => {
                 for (idx, view) in views.iter().enumerate() {
-                    validate_view(idx, view)?;
+                    validate_view(idx, view, true)?;
                 }
             }
         }

vortex-array/src/arrays/varbinview/tests.rs

@@ -1,11 +1,24 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
+#![allow(clippy::clone_on_ref_ptr)]
 
+use std::sync::Arc;
+
+use vortex_buffer::BitBuffer;
+use vortex_buffer::Buffer;
+use vortex_buffer::ByteBuffer;
+use vortex_error::VortexResult;
+
+use crate::IntoArray;
 use crate::VortexSessionExecute;
 use crate::array_session;
+use crate::arrays::BoolArray;
 use crate::arrays::VarBinViewArray;
 use crate::arrays::varbinview::BinaryView;
 use crate::assert_arrays_eq;
+use crate::dtype::DType;
+use crate::dtype::Nullability;
+use crate::validity::Validity;
 
 #[test]
 pub fn varbin_view() {
@@ -49,3 +62,44 @@ pub fn binary_view_size_and_alignment() {
     assert_eq!(size_of::<BinaryView>(), 16);
     assert_eq!(align_of::<BinaryView>(), 16);
 }
+
+// Null views in VarBinView are validated
+#[test]
+pub fn binary_view_null_view() {
+    let views = Buffer::<BinaryView>::copy_from(vec![
+        BinaryView::new_inlined(b"ololo"),
+        BinaryView::new_ref(14, *b"hell", 0, 0),
+        BinaryView::new_ref(13, *b"AAAA", 0xDEAD_BEEF, 0xF000_0000),
+    ]);
+    let data = b"hello world ololo";
+
+    let validity = BitBuffer::from_iter([true, true, false]);
+    let validity = BoolArray::new(validity, Validity::NonNullable);
+    let validity = Validity::Array(validity.into_array());
+    let buffers = Arc::new([ByteBuffer::from(data.to_vec())]);
+    let dtype = DType::Utf8(Nullability::Nullable);
+
+    let array = VarBinViewArray::try_new(views.clone(), buffers.clone(), dtype.clone(), validity);
+    assert!(array.is_err());
+    let array = VarBinViewArray::try_new(views, buffers, dtype, Validity::AllInvalid);
+    assert!(array.is_err());
+}
+
+/// Validation of Null views in VarBinView doesn't check prefix and contents
+#[test]
+pub fn binary_view_null_view_in_bounds() -> VortexResult<()> {
+    let data = b"hello world foo\xFF\xFE\xFD";
+    let buffers = Arc::new([ByteBuffer::from(data.to_vec())]);
+    let dtype = DType::Utf8(Nullability::Nullable);
+
+    let validity = BoolArray::new(BitBuffer::from_iter([true, false]), Validity::NonNullable);
+    let validity = Validity::Array(validity.into_array());
+
+    let valid_row = BinaryView::new_ref(15, *b"hell", 0, 0);
+    let garbage_row = BinaryView::new_ref(13, *b"XXXX", 0, 5);
+
+    let views = Buffer::<BinaryView>::copy_from(vec![valid_row, garbage_row]);
+    VarBinViewArray::try_new(views.clone(), buffers.clone(), dtype.clone(), validity)?;
+    VarBinViewArray::try_new(views, buffers, dtype, Validity::AllInvalid)?;
+    Ok(())
+}