remove ListViewShape and replace with is_zctl

Signed-off-by: Connor Tsui <[email protected]>

Author: connortsui20

vortex-array/src/arrays/chunked/vtable/canonical.rs

@@ -6,8 +6,7 @@ use vortex_dtype::{DType, Nullability, PType, StructFields};
 use vortex_error::VortexExpect;
 
 use crate::arrays::{
-    ChunkedArray, ChunkedVTable, ListViewArray, ListViewRebuildMode, ListViewShape, PrimitiveArray,
-    StructArray,
+    ChunkedArray, ChunkedVTable, ListViewArray, ListViewRebuildMode, PrimitiveArray, StructArray,
 };
 use crate::builders::{ArrayBuilder, builder_with_capacity};
 use crate::compute::cast;
@@ -166,14 +165,14 @@ fn swizzle_list_chunks(
 
     // Since we made sure that all chunk were zero-copyable to list above, we know that the final
     // output is also zero-copyable to a list.
-    let shape = ListViewShape::as_zero_copy_to_list();
+    let is_zctl = true;
 
     // SAFETY:
     // - `offsets` and `sizes` are non-nullable u64 arrays of the same length
     // - Each `offset[i] + size[i]` list view is within bounds of elements array because it came
     //   from valid chunks
     // - Validity came from the outer chunked array so it must have the same length
-    unsafe { ListViewArray::new_unchecked(chunked_elements, offsets, sizes, validity, shape) }
+    unsafe { ListViewArray::new_unchecked(chunked_elements, offsets, sizes, validity, is_zctl) }
 }
 
 #[cfg(test)]

vortex-array/src/arrays/constant/vtable/canonical.rs

@@ -16,7 +16,7 @@ use crate::arrays::constant::ConstantArray;
 use crate::arrays::primitive::PrimitiveArray;
 use crate::arrays::{
     BoolArray, ConstantVTable, DecimalArray, ExtensionArray, FixedSizeListArray, ListViewArray,
-    ListViewShape, NullArray, StructArray, VarBinViewArray, smallest_decimal_value_type,
+    NullArray, StructArray, VarBinViewArray, smallest_decimal_value_type,
 };
 use crate::builders::builder_with_capacity;
 use crate::validity::Validity;
@@ -253,14 +253,13 @@ fn constant_canonical_list_array(scalar: &Scalar, len: usize) -> ListViewArray {
     debug_assert!(!offsets.dtype().is_nullable());
     debug_assert!(!sizes.dtype().is_nullable());
 
-    // Since everything is pointing to the same exact thing, the offsets are sorted and there are no
-    // gaps in the shape.
-    let shape = ListViewShape::as_zero_copy_to_list().with_no_overlaps(false);
+    // Since all views are pointing to the same exact thing, it is not zero-copyable to `ListArray`.
+    let is_zctl = false;
 
     // SAFETY: All views point to the same range [0, list.len()) in the elements array.
     // The elements array contains `len` copies of the same value, offsets are all 0,
     // and sizes are all equal to the list length. The validity matches the scalar's nullability.
-    unsafe { ListViewArray::new_unchecked(elements, offsets, sizes, validity, shape) }
+    unsafe { ListViewArray::new_unchecked(elements, offsets, sizes, validity, is_zctl) }
 }
 
 fn constant_canonical_fixed_size_list_array(

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

@@ -269,6 +269,11 @@ impl ListArray {
         &self.elements
     }
 
+    // TODO(connor)[ListView]: Create 2 functions `reset_offsets` and `recursive_reset_offsets`,
+    // where `reset_offsets` is infallible.
+    // Also, `reset_offsets` can be made more efficient by replacing `sub_scalar` with a match on
+    // the offset type and manual subtraction and fast path where `offsets[0] == 0`.
+
     /// Create a copy of this array by adjusting `offsets` to start at `0` and removing elements not
     /// referenced by the `offsets`.
     pub fn reset_offsets(&self, recurse: bool) -> VortexResult<Self> {

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

@@ -7,7 +7,7 @@ use num_traits::AsPrimitive;
 use vortex_dtype::{DType, IntegerPType, match_each_integer_ptype};
 use vortex_error::{VortexExpect, VortexResult, vortex_bail, vortex_ensure, vortex_err};
 
-use crate::arrays::{ListViewShape, PrimitiveArray, PrimitiveVTable};
+use crate::arrays::{PrimitiveArray, PrimitiveVTable, bool};
 use crate::stats::ArrayStats;
 use crate::validity::Validity;
 use crate::{Array, ArrayRef, ToCanonical};
@@ -37,7 +37,7 @@ use crate::{Array, ArrayRef, ToCanonical};
 /// # Examples
 ///
 /// ```
-/// # use vortex_array::arrays::{ListViewArray, ListViewShape, PrimitiveArray};
+/// # use vortex_array::arrays::{ListViewArray, PrimitiveArray};
 /// # use vortex_array::validity::Validity;
 /// # use vortex_array::IntoArray;
 /// # use vortex_buffer::buffer;
@@ -55,9 +55,7 @@ use crate::{Array, ArrayRef, ToCanonical};
 ///     offsets.into_array(),
 ///     sizes.into_array(),
 ///     Validity::NonNullable,
-///     ListViewShape::as_zero_copy_to_list()
-///         .with_sorted_offsets(false)
-///         .with_no_overlaps(false),
+///     false, // NOT zero-copyable to a `ListArray`.
 /// ).unwrap();
 ///
 /// assert_eq!(list_view.len(), 3);
@@ -97,12 +95,11 @@ pub struct ListViewArray {
     /// we want to access.
     sizes: ArrayRef,
 
-    /// The "shape" of the `ListViewArray` data.
+    // TODO(connor): More docs. Also we need the n+1 memory allocation optimization.
+    /// A flag denoting if the array is zero-copyable* to a [`ListArray`].
     ///
     /// We use this information to help us more efficiently rebuild / compact our data.
-    ///
-    /// See the documentation for [`ListViewShape`] for more information.
-    shape: ListViewShape,
+    is_zero_copy_to_list: bool,
 
     /// The validity / null map of the array.
     ///
@@ -126,10 +123,10 @@ impl ListViewArray {
         offsets: ArrayRef,
         sizes: ArrayRef,
         validity: Validity,
-        shape: ListViewShape,
+        is_zctl: bool,
     ) -> Self {
-        Self::try_new(elements, offsets, sizes, validity, shape)
-            .vortex_expect("ListViewArray construction failed")
+        Self::try_new(elements, offsets, sizes, validity, is_zctl)
+            .vortex_expect("`ListViewArray` construction failed")
     }
 
     /// Constructs a new `ListViewArray`.
@@ -143,12 +140,12 @@ impl ListViewArray {
         offsets: ArrayRef,
         sizes: ArrayRef,
         validity: Validity,
-        shape: ListViewShape,
+        is_zctl: bool,
     ) -> VortexResult<Self> {
-        Self::validate(&elements, &offsets, &sizes, &validity, shape)?;
+        Self::validate(&elements, &offsets, &sizes, &validity, is_zctl)?;
 
         // SAFETY: validate ensures all invariants are met.
-        Ok(unsafe { Self::new_unchecked(elements, offsets, sizes, validity, shape) })
+        Ok(unsafe { Self::new_unchecked(elements, offsets, sizes, validity, is_zctl) })
     }
 
     /// Creates a new [`ListViewArray`] without validation.
@@ -163,26 +160,27 @@ impl ListViewArray {
     /// - For each `i`, `offsets[i] + sizes[i]` must not overflow and must be `<= elements.len()`
     ///   (even if the corresponding view is defined as null by the validity array).
     /// - If validity is an array, its length must equal `offsets.len()`.
-    /// - The `shape` that is passed in correctly describes the shape of the `ListViewArray` data.
+    /// - If the `is_zctl` flag is true, then the [`ListViewArray`] is zero-copyable to a
+    ///   [`ListArray`].
     pub unsafe fn new_unchecked(
         elements: ArrayRef,
         offsets: ArrayRef,
         sizes: ArrayRef,
         validity: Validity,
-        shape: ListViewShape,
+        is_zctl: bool,
     ) -> Self {
-        #[cfg(debug_assertions)]
-        Self::validate(&elements, &offsets, &sizes, &validity, shape)
-            .vortex_expect("[Debug Assertion]: Invalid `ListViewArray` parameters");
-
-        Self {
-            dtype: DType::List(Arc::new(elements.dtype().clone()), validity.nullability()),
-            elements,
-            offsets,
-            sizes,
-            validity,
-            shape,
-            stats_set: Default::default(),
+        if cfg!(debug_assertions) {
+            Self::new(elements, offsets, sizes, validity, is_zctl)
+        } else {
+            Self {
+                dtype: DType::List(Arc::new(elements.dtype().clone()), validity.nullability()),
+                elements,
+                offsets,
+                sizes,
+                validity,
+                is_zero_copy_to_list: is_zctl,
+                stats_set: Default::default(),
+            }
         }
     }
 
@@ -192,7 +190,7 @@ impl ListViewArray {
         offsets: &dyn Array,
         sizes: &dyn Array,
         validity: &Validity,
-        shape: ListViewShape,
+        is_zctl: bool,
     ) -> VortexResult<()> {
         // Check that offsets and sizes are integer arrays and non-nullable.
         vortex_ensure!(
@@ -255,8 +253,10 @@ impl ListViewArray {
             })
         });
 
-        // Validate the `ListViewShape`.
-        validate_shape(shape, elements, offsets_primitive, sizes_primitive)?;
+        // Validate the zero-copy to `ListArray` flag.
+        if is_zctl {
+            validate_zctl(elements, offsets_primitive, sizes_primitive)?;
+        }
 
         Ok(())
     }
@@ -336,11 +336,9 @@ impl ListViewArray {
         &self.elements
     }
 
-    /// Returns the shape of the `ListViewArray`.
-    ///
-    /// See the documentation of [`ListViewShape`] for more information.
-    pub fn shape(&self) -> ListViewShape {
-        self.shape
+    /// Returns true if the `ListViewArray` is zero-copyable to a [`ListArray`].
+    pub fn is_zero_copy_to_list(&self) -> bool {
+        self.is_zero_copy_to_list
     }
 }
 
@@ -394,25 +392,19 @@ where
     Ok(())
 }
 
-/// Helper function to validate if the [`ListViewShape`] correctly describes the data.
-fn validate_shape(
-    shape: ListViewShape,
+/// Helper function to validate if the [`ListViewArray`] components are actually zero-copyable to
+/// [`ListArray`].
+fn validate_zctl(
     elements: &dyn Array,
     offsets_primitive: PrimitiveArray,
     sizes_primitive: PrimitiveArray,
 ) -> VortexResult<()> {
-    if shape.has_sorted_offsets() {
-        // Offsets must be sorted (but not strictly sorted, zero-length lists are allowed), even
-        // if there are null views.
-        if let Some(is_sorted) = offsets_primitive.statistics().compute_is_sorted() {
-            vortex_ensure!(is_sorted, "offsets must be sorted");
-        } else {
-            vortex_bail!("offsets must report is_sorted statistic");
-        }
-    }
-
-    if !(shape.has_no_gaps() || shape.has_no_overlaps()) {
-        return Ok(());
+    // Offsets must be sorted (but not strictly sorted, zero-length lists are allowed), even
+    // if there are null views.
+    if let Some(is_sorted) = offsets_primitive.statistics().compute_is_sorted() {
+        vortex_ensure!(is_sorted, "offsets must be sorted");
+    } else {
+        vortex_bail!("offsets must report is_sorted statistic");
     }
 
     let mut element_references = vec![0u8; elements.len()];
@@ -426,7 +418,7 @@ fn validate_shape(
         let sizes_slice = sizes_primitive.as_slice::<S>();
 
         // Note that we ignore nulls here, as the "null" view metadata must still maintain the same
-        // invariants as non-null views, even for a `ListViewShape` information.
+        // invariants as non-null views, even for a `bool` information.
         for i in 0..offsets_slice.len() {
             let offset: usize = offsets_slice[i].as_();
             let size: usize = sizes_slice[i].as_();
@@ -442,28 +434,24 @@ fn validate_shape(
         })
     });
 
-    if shape.has_no_gaps() {
-        // Allow leading and trailing unreferenced elements, but not gaps in the middle.
-        let leftmost_used = element_references
-            .iter()
-            .position(|&references| references != 0);
-        let rightmost_used = element_references
-            .iter()
-            .rposition(|&references| references != 0);
+    // Allow leading and trailing unreferenced elements, but not gaps in the middle.
+    let leftmost_used = element_references
+        .iter()
+        .position(|&references| references != 0);
+    let rightmost_used = element_references
+        .iter()
+        .rposition(|&references| references != 0);
 
-        if let (Some(first_ref), Some(last_ref)) = (leftmost_used, rightmost_used) {
-            vortex_ensure!(
-                element_references[first_ref..=last_ref]
-                    .iter()
-                    .all(|&references| references != 0),
-                "found gap in elements array between first and last referenced elements"
-            );
-        }
+    if let (Some(first_ref), Some(last_ref)) = (leftmost_used, rightmost_used) {
+        vortex_ensure!(
+            element_references[first_ref..=last_ref]
+                .iter()
+                .all(|&references| references != 0),
+            "found gap in elements array between first and last referenced elements"
+        );
     }
 
-    if shape.has_no_overlaps() {
-        vortex_ensure!(element_references.iter().all(|&references| references <= 1))
-    }
+    vortex_ensure!(element_references.iter().all(|&references| references <= 1));
 
     Ok(())
 }

vortex-array/src/arrays/listview/compute/cast.rs

@@ -31,7 +31,7 @@ impl CastKernel for ListViewVTable {
                     array.offsets().clone(),
                     array.sizes().clone(),
                     validity,
-                    array.shape(),
+                    array.is_zero_copy_to_list(),
                 )
             }
             .to_array(),

vortex-array/src/arrays/listview/compute/filter.rs

@@ -49,9 +49,8 @@ impl FilterKernel for ListViewVTable {
         let new_offsets = compute::filter(offsets.as_ref(), selection_mask)?;
         let new_sizes = compute::filter(sizes.as_ref(), selection_mask)?;
 
-        // Filters keep scalars in order, and it cannot cause overlaps to form. However, filters
-        // **will** create gaps of unused elements.
-        let new_shape = array.shape().with_no_gaps(false);
+        // Filters will create gaps of unused elements.
+        let is_zctl = false;
 
         // SAFETY: Filter operation maintains all `ListViewArray` invariants:
         // - Offsets and sizes are derived from existing valid child arrays.
@@ -63,7 +62,7 @@ impl FilterKernel for ListViewVTable {
                 new_offsets,
                 new_sizes,
                 new_validity,
-                new_shape,
+                is_zctl,
             )
         };
 

vortex-array/src/arrays/listview/compute/mask.rs

@@ -16,7 +16,7 @@ impl MaskKernel for ListViewVTable {
             array.offsets().clone(),
             array.sizes().clone(),
             array.validity().mask(mask),
-            array.shape(),
+            array.is_zero_copy_to_list(),
         )
         .map(|a| a.into_array())
     }

vortex-array/src/arrays/listview/compute/take.rs

@@ -6,7 +6,7 @@ use vortex_dtype::{Nullability, match_each_integer_ptype};
 use vortex_error::VortexResult;
 use vortex_scalar::Scalar;
 
-use crate::arrays::{ListViewArray, ListViewShape, ListViewVTable};
+use crate::arrays::{ListViewArray, ListViewVTable};
 use crate::compute::{self, TakeKernel, TakeKernelAdapter};
 use crate::vtable::ValidityHelper;
 use crate::{Array, ArrayRef, IntoArray, register_kernel};
@@ -66,7 +66,7 @@ impl TakeKernel for ListViewVTable {
 
         // Take can reorder offsets, create gaps, and may introduce overlaps if the `indices`
         // contain duplicates.
-        let new_shape = ListViewShape::default();
+        let is_zctl = false;
 
         // SAFETY: Take operation maintains all `ListViewArray` invariants:
         // - `new_offsets` and `new_sizes` are derived from existing valid child arrays.
@@ -80,7 +80,7 @@ impl TakeKernel for ListViewVTable {
                 new_offsets,
                 new_sizes,
                 new_validity,
-                new_shape,
+                is_zctl,
             )
         };