From 31e41b447e7ac409e64e4b4aac471867af025616 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Fri, 3 Apr 2026 16:30:18 -0400 Subject: [PATCH 1/2] Fix Array::validity Signed-off-by: Nicholas Gates --- encodings/alp/src/alp/array.rs | 7 ++- encodings/alp/src/alp/compress.rs | 2 +- encodings/alp/src/alp/decompress.rs | 7 ++- encodings/alp/src/alp_rd/mod.rs | 7 ++- encodings/bytebool/src/array.rs | 40 ++++++++------- encodings/bytebool/src/compute.rs | 12 ++--- encodings/bytebool/src/slice.rs | 2 +- encodings/datetime-parts/src/compress.rs | 14 +++-- .../src/decimal_byte_parts/mod.rs | 2 +- .../src/bitpacking/array/bitpack_compress.rs | 6 +-- .../fastlanes/src/bitpacking/compute/cast.rs | 2 +- .../src/bitpacking/compute/filter.rs | 4 +- .../fastlanes/src/bitpacking/compute/slice.rs | 4 +- .../fastlanes/src/bitpacking/compute/take.rs | 6 +-- .../fastlanes/src/bitpacking/vtable/mod.rs | 3 +- .../src/delta/array/delta_compress.rs | 5 +- .../src/delta/array/delta_decompress.rs | 2 +- .../fastlanes/src/for/array/for_decompress.rs | 2 +- .../fastlanes/src/for/compute/is_sorted.rs | 2 +- .../fastlanes/src/rle/array/rle_compress.rs | 16 ++++-- encodings/fsst/src/array.rs | 7 ++- encodings/fsst/src/canonical.rs | 2 +- encodings/fsst/src/compute/like.rs | 2 +- encodings/parquet-variant/src/array.rs | 20 ++++++-- encodings/parquet-variant/src/kernel.rs | 6 +-- encodings/parquet-variant/src/operations.rs | 2 +- encodings/parquet-variant/src/validity.rs | 13 +++-- encodings/parquet-variant/src/vtable.rs | 12 ++--- encodings/pco/src/array.rs | 2 +- encodings/runend/src/arrow.rs | 2 +- encodings/runend/src/compress.rs | 5 +- encodings/runend/src/compute/take.rs | 3 +- encodings/sparse/src/canonical.rs | 1 + encodings/zigzag/src/compress.rs | 7 ++- encodings/zstd/src/array.rs | 4 +- fuzz/src/array/fill_null.rs | 20 ++++++-- fuzz/src/array/mask.rs | 14 ++--- .../src/aggregate_fn/accumulator_grouped.rs | 10 ++-- vortex-array/src/array/typed.rs | 17 ++----- vortex-array/src/array/view.rs | 18 +++---- vortex-array/src/array/vtable/validity.rs | 19 +------ vortex-array/src/arrays/bool/array.rs | 2 +- vortex-array/src/arrays/bool/compute/cast.rs | 2 +- .../src/arrays/bool/compute/fill_null.rs | 2 +- .../src/arrays/bool/compute/filter.rs | 2 +- vortex-array/src/arrays/bool/compute/mask.rs | 2 +- vortex-array/src/arrays/bool/compute/rules.rs | 2 +- vortex-array/src/arrays/bool/compute/slice.rs | 2 +- vortex-array/src/arrays/bool/compute/take.rs | 2 +- vortex-array/src/arrays/bool/patch.rs | 4 +- .../src/arrays/constant/vtable/canonical.rs | 15 +++--- vortex-array/src/arrays/datetime/test.rs | 2 + vortex-array/src/arrays/decimal/array.rs | 2 +- .../src/arrays/decimal/compute/between.rs | 5 +- .../src/arrays/decimal/compute/cast.rs | 8 +-- .../src/arrays/decimal/compute/fill_null.rs | 2 +- .../src/arrays/decimal/compute/mask.rs | 2 +- .../src/arrays/decimal/compute/rules.rs | 4 +- .../src/arrays/decimal/compute/take.rs | 2 +- vortex-array/src/arrays/decimal/utils.rs | 4 +- .../src/arrays/filter/execute/bool.rs | 3 +- .../src/arrays/filter/execute/decimal.rs | 8 ++- .../arrays/filter/execute/fixed_size_list.rs | 7 ++- .../src/arrays/filter/execute/listview.rs | 7 ++- .../src/arrays/filter/execute/primitive.rs | 5 +- .../src/arrays/filter/execute/struct_.rs | 7 ++- .../arrays/fixed_size_list/compute/cast.rs | 2 +- .../arrays/fixed_size_list/compute/mask.rs | 2 +- .../arrays/fixed_size_list/compute/slice.rs | 2 +- .../arrays/fixed_size_list/compute/take.rs | 2 +- vortex-array/src/arrays/list/compute/cast.rs | 2 +- .../src/arrays/list/compute/filter.rs | 2 +- vortex-array/src/arrays/list/compute/mask.rs | 2 +- vortex-array/src/arrays/list/compute/slice.rs | 2 +- vortex-array/src/arrays/list/compute/take.rs | 4 +- .../src/arrays/listview/compute/cast.rs | 2 +- .../src/arrays/listview/compute/mask.rs | 2 +- .../src/arrays/listview/compute/rules.rs | 2 +- .../src/arrays/listview/compute/slice.rs | 2 +- .../src/arrays/listview/compute/take.rs | 2 +- .../src/arrays/listview/conversion.rs | 12 +++-- vortex-array/src/arrays/listview/rebuild.rs | 16 +++--- .../src/arrays/masked/compute/filter.rs | 2 +- .../src/arrays/masked/compute/mask.rs | 1 + .../src/arrays/masked/compute/slice.rs | 2 +- .../src/arrays/masked/compute/take.rs | 2 +- vortex-array/src/arrays/masked/execute.rs | 14 ++--- vortex-array/src/arrays/masked/tests.rs | 9 +++- .../src/arrays/primitive/array/accessor.rs | 4 +- .../src/arrays/primitive/array/cast.rs | 17 ++++--- .../src/arrays/primitive/array/patch.rs | 4 +- .../src/arrays/primitive/array/top_value.rs | 2 +- .../src/arrays/primitive/compute/between.rs | 5 +- .../src/arrays/primitive/compute/cast.rs | 14 ++--- .../src/arrays/primitive/compute/fill_null.rs | 2 +- .../src/arrays/primitive/compute/mask.rs | 2 +- .../src/arrays/primitive/compute/rules.rs | 2 +- .../src/arrays/primitive/compute/slice.rs | 2 +- .../src/arrays/primitive/compute/take/mod.rs | 4 +- vortex-array/src/arrays/struct_/array.rs | 2 +- .../src/arrays/struct_/compute/cast.rs | 2 +- .../src/arrays/struct_/compute/mask.rs | 2 +- .../src/arrays/struct_/compute/rules.rs | 6 +-- .../src/arrays/struct_/compute/slice.rs | 2 +- .../src/arrays/struct_/compute/take.rs | 2 +- .../src/arrays/struct_/compute/zip.rs | 4 +- vortex-array/src/arrays/varbin/accessor.rs | 6 ++- .../src/arrays/varbin/compute/cast.rs | 2 +- .../src/arrays/varbin/compute/compare.rs | 2 +- .../src/arrays/varbin/compute/filter.rs | 2 +- .../src/arrays/varbin/compute/mask.rs | 2 +- .../src/arrays/varbin/compute/slice.rs | 2 +- .../src/arrays/varbinview/accessor.rs | 7 ++- .../src/arrays/varbinview/compute/cast.rs | 2 +- .../src/arrays/varbinview/compute/mask.rs | 2 +- .../src/arrays/varbinview/compute/slice.rs | 2 +- .../src/arrays/varbinview/compute/take.rs | 2 +- vortex-array/src/arrow/executor/byte.rs | 2 +- vortex-array/src/arrow/executor/byte_view.rs | 2 +- .../src/arrow/executor/fixed_size_list.rs | 2 +- vortex-array/src/arrow/executor/list.rs | 2 +- vortex-array/src/builders/bool.rs | 3 +- vortex-array/src/builders/fixed_size_list.rs | 51 ++++++++++--------- vortex-array/src/builders/list.rs | 15 ++++-- vortex-array/src/builders/listview.rs | 9 ++-- vortex-array/src/builders/primitive.rs | 7 +-- vortex-array/src/canonical.rs | 2 +- vortex-array/src/patches.rs | 10 ++-- vortex-array/src/scalar_fn/fns/get_item.rs | 2 +- .../src/scalar_fn/fns/list_contains/mod.rs | 8 +-- vortex-array/src/scalar_fn/fns/not/mod.rs | 2 +- vortex-array/src/scalar_fn/fns/pack.rs | 4 +- vortex-btrblocks/src/schemes/decimal.rs | 2 +- vortex-btrblocks/src/schemes/integer.rs | 2 +- vortex-btrblocks/src/schemes/string.rs | 2 +- vortex-compressor/src/builtins/constant.rs | 4 +- vortex-compressor/src/builtins/dict/float.rs | 5 +- .../src/builtins/dict/integer.rs | 5 +- vortex-compressor/src/compressor.rs | 6 +-- vortex-cuda/src/arrow/canonical.rs | 2 +- vortex-cuda/src/kernel/arrays/dict.rs | 4 +- vortex-cuda/src/kernel/patches/mod.rs | 2 +- vortex-duckdb/src/exporter/bool.rs | 2 +- vortex-duckdb/src/exporter/mod.rs | 2 +- vortex-duckdb/src/exporter/primitive.rs | 2 +- vortex-layout/src/layouts/struct_/reader.rs | 2 +- .../common_encoding_tree_throughput.rs | 5 +- vortex/src/lib.rs | 4 +- 148 files changed, 460 insertions(+), 343 deletions(-) diff --git a/encodings/alp/src/alp/array.rs b/encodings/alp/src/alp/array.rs index 93dfb8d3932..feff33c6269 100644 --- a/encodings/alp/src/alp/array.rs +++ b/encodings/alp/src/alp/array.rs @@ -571,6 +571,7 @@ mod tests { use vortex_array::arrays::PrimitiveArray; use vortex_array::assert_arrays_eq; use vortex_array::session::ArraySession; + use vortex_error::VortexExpect; use vortex_session::VortexSession; use super::*; @@ -775,7 +776,11 @@ mod tests { for idx in 0..slice_len { let expected_value = values[slice_start + idx]; - let result_valid = result_primitive.validity().is_valid(idx).unwrap(); + let result_valid = result_primitive + .validity() + .vortex_expect("result validity should be derivable") + .is_valid(idx) + .unwrap(); assert_eq!( result_valid, expected_value.is_some(), diff --git a/encodings/alp/src/alp/compress.rs b/encodings/alp/src/alp/compress.rs index a55baf02698..7fda471bbda 100644 --- a/encodings/alp/src/alp/compress.rs +++ b/encodings/alp/src/alp/compress.rs @@ -66,7 +66,7 @@ where let (exponents, encoded, exceptional_positions, exceptional_values, mut chunk_offsets) = T::encode(values_slice, exponents); - let encoded_array = PrimitiveArray::new(encoded, values.validity()).into_array(); + let encoded_array = PrimitiveArray::new(encoded, values.validity()?).into_array(); let validity = values.validity_mask()?; // exceptional_positions may contain exceptions at invalid positions (which contain garbage diff --git a/encodings/alp/src/alp/decompress.rs b/encodings/alp/src/alp/decompress.rs index e118b9705cd..ea52e362efe 100644 --- a/encodings/alp/src/alp/decompress.rs +++ b/encodings/alp/src/alp/decompress.rs @@ -11,6 +11,7 @@ use vortex_array::dtype::DType; use vortex_array::match_each_unsigned_integer_ptype; use vortex_array::patches::Patches; use vortex_buffer::BufferMut; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use crate::ALPArray; @@ -102,7 +103,9 @@ fn decompress_chunked_core( patches: &Patches, dtype: DType, ) -> PrimitiveArray { - let validity = encoded.validity(); + let validity = encoded + .validity() + .vortex_expect("ALP validity should be derivable"); let ptype = dtype.as_ptype(); let array_len = encoded.len(); let offset_within_chunk = patches.offset_within_chunk().unwrap_or(0); @@ -152,7 +155,7 @@ fn decompress_unchunked_core( dtype: DType, ctx: &mut ExecutionCtx, ) -> VortexResult { - let validity = encoded.validity(); + let validity = encoded.validity()?; let ptype = dtype.as_ptype(); let decoded = match_each_alp_float_ptype!(ptype, |T| { diff --git a/encodings/alp/src/alp_rd/mod.rs b/encodings/alp/src/alp_rd/mod.rs index d1bf8b9cdc8..6202dd799e7 100644 --- a/encodings/alp/src/alp_rd/mod.rs +++ b/encodings/alp/src/alp_rd/mod.rs @@ -227,7 +227,12 @@ impl RDEncoder { } // Bit-pack down the encoded left-parts array that have been dictionary encoded. - let primitive_left = PrimitiveArray::new(left_parts, array.validity()); + let primitive_left = PrimitiveArray::new( + left_parts, + array + .validity() + .vortex_expect("ALP RD validity should be derivable"), + ); // SAFETY: by construction, all values in left_parts can be packed to left_bit_width. let packed_left = unsafe { bitpack_encode_unchecked(primitive_left, left_bit_width as _) diff --git a/encodings/bytebool/src/array.rs b/encodings/bytebool/src/array.rs index 48448ba7b57..44a10177999 100644 --- a/encodings/bytebool/src/array.rs +++ b/encodings/bytebool/src/array.rs @@ -17,14 +17,15 @@ use vortex_array::Precision; use vortex_array::arrays::BoolArray; use vortex_array::buffer::BufferHandle; use vortex_array::dtype::DType; +use vortex_array::dtype::Nullability; use vortex_array::scalar::Scalar; use vortex_array::serde::ArrayChildren; use vortex_array::validity::Validity; use vortex_array::vtable; use vortex_array::vtable::OperationsVTable; use vortex_array::vtable::VTable; -use vortex_array::vtable::ValidityHelper; -use vortex_array::vtable::ValidityVTableFromValidityHelper; +use vortex_array::vtable::ValidityVTable; +use vortex_array::vtable::child_to_validity; use vortex_array::vtable::validity_to_child; use vortex_buffer::BitBuffer; use vortex_buffer::ByteBuffer; @@ -43,24 +44,25 @@ impl VTable for ByteBool { type ArrayData = ByteBoolData; type OperationsVTable = Self; - type ValidityVTable = ValidityVTableFromValidityHelper; + type ValidityVTable = Self; fn id(&self) -> ArrayId { Self::ID } fn validate(&self, data: &Self::ArrayData, dtype: &DType, len: usize) -> VortexResult<()> { - ByteBoolData::validate(data.buffer(), data.validity(), dtype, len) + let validity = data.validity(); + ByteBoolData::validate(data.buffer(), &validity, dtype, len) } fn array_hash(array: &ByteBoolData, state: &mut H, precision: Precision) { array.buffer.array_hash(state, precision); - array.validity.array_hash(state, precision); + array.validity().array_hash(state, precision); } fn array_eq(array: &ByteBoolData, other: &ByteBoolData, precision: Precision) -> bool { array.buffer.array_eq(&other.buffer, precision) - && array.validity.array_eq(&other.validity, precision) + && array.validity().array_eq(&other.validity(), precision) } fn nbuffers(_array: ArrayView<'_, Self>) -> usize { @@ -132,10 +134,6 @@ impl VTable for ByteBool { NUM_SLOTS, slots.len() ); - array.validity = match &slots[VALIDITY_SLOT] { - Some(arr) => Validity::Array(arr.clone()), - None => Validity::from(array.validity.nullability()), - }; array.slots = slots; Ok(()) } @@ -150,7 +148,7 @@ impl VTable for ByteBool { fn execute(array: Array, _ctx: &mut ExecutionCtx) -> VortexResult { let boolean_buffer = BitBuffer::from(array.as_slice()); - let validity = array.validity().clone(); + let validity = array.validity()?; Ok(ExecutionResult::done( BoolArray::new(boolean_buffer, validity).into_array(), )) @@ -174,7 +172,7 @@ pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = ["validity"]; #[derive(Clone, Debug)] pub struct ByteBoolData { buffer: BufferHandle, - validity: Validity, + nullability: Nullability, pub(super) slots: Vec>, } @@ -194,7 +192,7 @@ impl ByteBool { /// Construct a [`ByteBoolArray`] from a `Vec` and validity. pub fn from_vec>(data: Vec, validity: V) -> ByteBoolArray { let data = ByteBoolData::from_vec(data, validity); - let dtype = DType::Bool(data.validity.nullability()); + let dtype = DType::Bool(data.nullability); let len = data.len(); unsafe { Array::from_parts_unchecked(ArrayParts::new(ByteBool, dtype, len, data)) } } @@ -202,7 +200,7 @@ impl ByteBool { /// Construct a [`ByteBoolArray`] from optional bools. pub fn from_option_vec(data: Vec>) -> ByteBoolArray { let data = ByteBoolData::from(data); - let dtype = DType::Bool(data.validity.nullability()); + let dtype = DType::Bool(data.nullability); let len = data.len(); unsafe { Array::from_parts_unchecked(ArrayParts::new(ByteBool, dtype, len, data)) } } @@ -235,6 +233,10 @@ impl ByteBoolData { vec![validity_to_child(validity, len)] } + pub fn validity(&self) -> Validity { + child_to_validity(&self.slots[VALIDITY_SLOT], self.nullability) + } + pub fn new(buffer: BufferHandle, validity: Validity) -> Self { let length = buffer.len(); if let Some(vlen) = validity.maybe_len() @@ -249,7 +251,7 @@ impl ByteBoolData { let slots = Self::make_slots(&validity, length); Self { buffer, - validity, + nullability: validity.nullability(), slots, } } @@ -266,7 +268,7 @@ impl ByteBoolData { /// Returns the validity mask for this array. pub fn validity_mask(&self) -> Mask { - self.validity.to_mask(self.len()) + self.validity().to_mask(self.len()) } // TODO(ngates): deprecate construction from vec @@ -287,9 +289,9 @@ impl ByteBoolData { } } -impl ValidityHelper for ByteBoolData { - fn validity(&self) -> &Validity { - &self.validity +impl ValidityVTable for ByteBool { + fn validity(array: ArrayView<'_, ByteBool>) -> VortexResult { + Ok(array.data().validity()) } } diff --git a/encodings/bytebool/src/compute.rs b/encodings/bytebool/src/compute.rs index 904c8e5cf93..ebb0bdbc4bf 100644 --- a/encodings/bytebool/src/compute.rs +++ b/encodings/bytebool/src/compute.rs @@ -25,10 +25,7 @@ impl CastReduce for ByteBool { // If just changing nullability, we can optimize if array.dtype().eq_ignore_nullability(dtype) { - let new_validity = array - .validity() - .clone() - .cast_nullability(dtype.nullability(), array.len())?; + let new_validity = array.validity()?.cast_nullability(dtype.nullability(), array.len())?; return Ok(Some( ByteBool::new(array.buffer().clone(), new_validity).into_array(), @@ -45,10 +42,7 @@ impl MaskReduce for ByteBool { Ok(Some( ByteBool::new( array.buffer().clone(), - array - .validity() - .clone() - .and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) .into_array(), )) @@ -65,7 +59,7 @@ impl TakeExecute for ByteBool { let bools = array.as_slice(); // This handles combining validity from both source array and nullable indices - let validity = array.validity().take(&indices.clone().into_array())?; + let validity = array.validity()?.take(&indices.clone().into_array())?; let taken_bools = match_each_integer_ptype!(indices.ptype(), |I| { indices diff --git a/encodings/bytebool/src/slice.rs b/encodings/bytebool/src/slice.rs index 4c55f5bbd40..34ffa844b3f 100644 --- a/encodings/bytebool/src/slice.rs +++ b/encodings/bytebool/src/slice.rs @@ -16,7 +16,7 @@ impl SliceReduce for ByteBool { Ok(Some( ByteBool::new( array.buffer().slice(range.clone()), - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .into_array(), )) diff --git a/encodings/datetime-parts/src/compress.rs b/encodings/datetime-parts/src/compress.rs index 662edbf8196..6be0d24f3f1 100644 --- a/encodings/datetime-parts/src/compress.rs +++ b/encodings/datetime-parts/src/compress.rs @@ -51,7 +51,7 @@ pub fn split_temporal(array: TemporalArray) -> VortexResult { } Ok(TemporalParts { - days: PrimitiveArray::new(days, temporal_values.validity()).into_array(), + days: PrimitiveArray::new(days, temporal_values.validity()?).into_array(), seconds: seconds.into_array(), subseconds: subseconds.into_array(), }) @@ -83,6 +83,7 @@ mod tests { use vortex_array::extension::datetime::TimeUnit; use vortex_array::validity::Validity; use vortex_buffer::buffer; + use vortex_error::VortexExpect; use crate::TemporalParts; use crate::split_temporal; @@ -114,15 +115,22 @@ mod tests { assert!( days.to_primitive() .validity() + .vortex_expect("days validity should be derivable") .mask_eq(&validity, &mut ctx) .unwrap() ); assert!(matches!( - seconds.to_primitive().validity(), + seconds + .to_primitive() + .validity() + .vortex_expect("seconds validity should be derivable"), Validity::NonNullable )); assert!(matches!( - subseconds.to_primitive().validity(), + subseconds + .to_primitive() + .validity() + .vortex_expect("subseconds validity should be derivable"), Validity::NonNullable )); } diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs index 8773b2e8184..6623e554f36 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs @@ -292,7 +292,7 @@ fn to_canonical_decimal( .dtype() .as_decimal_opt() .vortex_expect("must be a decimal dtype"), - prim.validity(), + prim.validity()?, ) } .into_array() diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs index 4290e5cb44a..9b6fcbc9fd1 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs @@ -74,7 +74,7 @@ pub fn bitpack_encode( let bitpacked = BitPacked::try_new( BufferHandle::new_host(packed), array.ptype(), - array.validity(), + array.validity()?, patches, bit_width, array.len(), @@ -103,7 +103,7 @@ pub unsafe fn bitpack_encode_unchecked( let bitpacked = BitPacked::try_new( BufferHandle::new_host(packed), array.ptype(), - array.validity(), + array.validity()?, None, bit_width, array.len(), @@ -191,7 +191,7 @@ pub fn gather_patches( bit_width: u8, num_exceptions_hint: usize, ) -> VortexResult> { - let patch_validity = match parray.validity() { + let patch_validity = match parray.validity()? { Validity::NonNullable => Validity::NonNullable, _ => Validity::AllValid, }; diff --git a/encodings/fastlanes/src/bitpacking/compute/cast.rs b/encodings/fastlanes/src/bitpacking/compute/cast.rs index b7e6a7b63da..3ebb97c511b 100644 --- a/encodings/fastlanes/src/bitpacking/compute/cast.rs +++ b/encodings/fastlanes/src/bitpacking/compute/cast.rs @@ -15,7 +15,7 @@ impl CastReduce for BitPacked { fn cast(array: ArrayView<'_, Self>, dtype: &DType) -> VortexResult> { if array.dtype().eq_ignore_nullability(dtype) { let new_validity = array - .validity(array.dtype().nullability()) + .validity()? .cast_nullability(dtype.nullability(), array.len())?; return Ok(Some( BitPacked::try_new( diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index 5ebeaa4705c..b0d7629d089 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -73,7 +73,7 @@ impl FilterKernel for BitPacked { PrimitiveArray::from_buffer_handle( primitive.buffer_handle().clone(), array.dtype().as_ptype(), - primitive.validity(), + primitive.validity()?, ) } else { primitive @@ -112,7 +112,7 @@ fn filter_primitive_without_patches( selection: &Arc, ) -> VortexResult<(Buffer, Validity)> { let values = filter_with_indices(array.data(), selection.indices()); - let validity = array.validity().filter(&Mask::Values(selection.clone()))?; + let validity = array.validity()?.filter(&Mask::Values(selection.clone()))?; Ok((values.freeze(), validity)) } diff --git a/encodings/fastlanes/src/bitpacking/compute/slice.rs b/encodings/fastlanes/src/bitpacking/compute/slice.rs index cc9394953f4..0e7dc4d7e43 100644 --- a/encodings/fastlanes/src/bitpacking/compute/slice.rs +++ b/encodings/fastlanes/src/bitpacking/compute/slice.rs @@ -27,9 +27,7 @@ impl SliceReduce for BitPacked { BitPacked::try_new( array.packed().slice(encoded_start..encoded_stop), array.dtype().as_ptype(), - array - .validity(array.dtype().nullability()) - .slice(range.clone())?, + array.validity()?.slice(range.clone())?, array .patches(array.len()) .map(|p| p.slice(range.clone())) diff --git a/encodings/fastlanes/src/bitpacking/compute/take.rs b/encodings/fastlanes/src/bitpacking/compute/take.rs index 6d652c42b42..6a8415e293d 100644 --- a/encodings/fastlanes/src/bitpacking/compute/take.rs +++ b/encodings/fastlanes/src/bitpacking/compute/take.rs @@ -48,7 +48,7 @@ impl TakeExecute for BitPacked { // NOTE: we use the unsigned PType because all values in the BitPackedArray must // be non-negative (pre-condition of creating the BitPackedArray). let ptype: PType = PType::try_from(array.dtype())?; - let validity = array.validity(); + let validity = array.validity()?; let taken_validity = validity.take(indices)?; let indices = indices.clone().execute::(ctx)?; @@ -61,7 +61,7 @@ impl TakeExecute for BitPacked { PrimitiveArray::from_buffer_handle( taken.buffer_handle().clone(), ptype, - taken.validity(), + taken.validity()?, ) } else { taken @@ -142,7 +142,7 @@ fn take_primitive( PrimitiveArray::from_buffer_handle( primitive.buffer_handle().clone(), array.dtype().as_ptype(), - primitive.validity(), + primitive.validity()?, ) } else { PrimitiveArray::new(output, taken_validity) diff --git a/encodings/fastlanes/src/bitpacking/vtable/mod.rs b/encodings/fastlanes/src/bitpacking/vtable/mod.rs index 317788ce9c5..29741c89623 100644 --- a/encodings/fastlanes/src/bitpacking/vtable/mod.rs +++ b/encodings/fastlanes/src/bitpacking/vtable/mod.rs @@ -314,9 +314,10 @@ impl VTable for BitPacked { PATCH_VALUES_SLOT, PATCH_CHUNK_OFFSETS_SLOT ); + let validity = array.validity()?; require_validity!( array, - &array.validity(), + &validity, VALIDITY_SLOT => AnyCanonical ); diff --git a/encodings/fastlanes/src/delta/array/delta_compress.rs b/encodings/fastlanes/src/delta/array/delta_compress.rs index 0b79489246d..105004c0c71 100644 --- a/encodings/fastlanes/src/delta/array/delta_compress.rs +++ b/encodings/fastlanes/src/delta/array/delta_compress.rs @@ -22,14 +22,15 @@ pub fn delta_compress( array: &PrimitiveArray, ctx: &mut ExecutionCtx, ) -> VortexResult<(PrimitiveArray, PrimitiveArray)> { + let validity = array.validity()?; let (bases, deltas) = match_each_unsigned_integer_ptype!(array.ptype(), |T| { // Fill-forward null values so that transposed deltas at null positions remain // small. Without this, bitpacking may skip patches for null positions, and the // corrupted delta values propagate through the cumulative sum during decompression. - let filled = fill_forward_nulls(array.to_buffer::(), &array.validity()); + let filled = fill_forward_nulls(array.to_buffer::(), &validity); let (bases, deltas) = compress_primitive::(&filled); // TODO(robert): This can be avoided if we add TransposedBoolArray that performs index translation when necessary. - let validity = transpose_validity(&array.validity(), ctx)?; + let validity = transpose_validity(&validity, ctx)?; ( PrimitiveArray::new(bases, array.dtype().nullability().into()), PrimitiveArray::new(deltas, validity), diff --git a/encodings/fastlanes/src/delta/array/delta_decompress.rs b/encodings/fastlanes/src/delta/array/delta_decompress.rs index 9ac446a92fc..7539bcdb0e2 100644 --- a/encodings/fastlanes/src/delta/array/delta_decompress.rs +++ b/encodings/fastlanes/src/delta/array/delta_decompress.rs @@ -29,7 +29,7 @@ pub fn delta_decompress( let start = array.offset(); let end = start + array.len(); - let validity = untranspose_validity(&deltas.validity(), ctx)?; + let validity = untranspose_validity(&deltas.validity()?, ctx)?; let validity = validity.slice(start..end)?; Ok(match_each_unsigned_integer_ptype!(deltas.ptype(), |T| { diff --git a/encodings/fastlanes/src/for/array/for_decompress.rs b/encodings/fastlanes/src/for/array/for_decompress.rs index 82a84b73fa4..11471162285 100644 --- a/encodings/fastlanes/src/for/array/for_decompress.rs +++ b/encodings/fastlanes/src/for/array/for_decompress.rs @@ -58,7 +58,7 @@ pub fn decompress(array: &FoRArray, ctx: &mut ExecutionCtx) -> VortexResult(ctx)?; - let validity = encoded.validity(); + let validity = encoded.validity()?; Ok(match_each_integer_ptype!(ptype, |T| { let min = array diff --git a/encodings/fastlanes/src/for/compute/is_sorted.rs b/encodings/fastlanes/src/for/compute/is_sorted.rs index 95504cdb7c7..8e86dba1a2c 100644 --- a/encodings/fastlanes/src/for/compute/is_sorted.rs +++ b/encodings/fastlanes/src/for/compute/is_sorted.rs @@ -38,7 +38,7 @@ impl DynAggregateKernel for FoRIsSortedKernel { let unsigned_array = PrimitiveArray::from_buffer_handle( encoded.buffer_handle().clone(), encoded.ptype().to_unsigned(), - encoded.validity(), + encoded.validity()?, ) .into_array(); diff --git a/encodings/fastlanes/src/rle/array/rle_compress.rs b/encodings/fastlanes/src/rle/array/rle_compress.rs index 11270da5486..4663f13d182 100644 --- a/encodings/fastlanes/src/rle/array/rle_compress.rs +++ b/encodings/fastlanes/src/rle/array/rle_compress.rs @@ -12,6 +12,7 @@ use vortex_array::match_each_native_ptype; use vortex_array::validity::Validity; use vortex_buffer::BitBufferMut; use vortex_buffer::BufferMut; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use crate::FL_CHUNK_SIZE; @@ -37,7 +38,7 @@ where { // Fill-forward null values so the RLE encoder doesn't see garbage at null positions, // which would create spurious run boundaries and inflate the dictionary. - let values = fill_forward_nulls(array.to_buffer::(), &array.validity()); + let values = fill_forward_nulls(array.to_buffer::(), &array.validity()?); let len = values.len(); let padded_len = len.next_multiple_of(FL_CHUNK_SIZE); @@ -110,7 +111,10 @@ where /// Returns validity padded to the next 1024 chunk for a given array. fn padded_validity(array: &PrimitiveArray) -> Validity { - match array.validity() { + match array + .validity() + .vortex_expect("RLE validity should be derivable") + { Validity::NonNullable => Validity::NonNullable, Validity::AllValid => Validity::AllValid, Validity::AllInvalid => Validity::AllInvalid, @@ -142,6 +146,7 @@ mod tests { use vortex_array::dtype::half::f16; use vortex_buffer::Buffer; use vortex_buffer::buffer; + use vortex_error::VortexExpect; use super::*; @@ -255,7 +260,12 @@ mod tests { let primitive = values.clone().into_array().to_primitive(); let result = RLEData::encode(&primitive).unwrap(); let decoded = result.as_array().to_primitive(); - let expected = PrimitiveArray::new(values, primitive.validity()); + let expected = PrimitiveArray::new( + values, + primitive + .validity() + .vortex_expect("primitive validity should be derivable"), + ); assert_arrays_eq!(decoded, expected); } diff --git a/encodings/fsst/src/array.rs b/encodings/fsst/src/array.rs index d716bc7608b..417ea7b420e 100644 --- a/encodings/fsst/src/array.rs +++ b/encodings/fsst/src/array.rs @@ -485,7 +485,12 @@ impl FSSTData { as Box Compressor + Send>)); let codes_array = codes.clone().into_array(); let codes_offsets_slot = Some(codes.offsets().clone()); - let codes_validity_slot = validity_to_child(&codes.validity(), codes.len()); + let codes_validity_slot = validity_to_child( + &codes + .validity() + .vortex_expect("FSST codes validity should be derivable"), + codes.len(), + ); Self { symbols, diff --git a/encodings/fsst/src/canonical.rs b/encodings/fsst/src/canonical.rs index 64fb8ea2117..d534ec22173 100644 --- a/encodings/fsst/src/canonical.rs +++ b/encodings/fsst/src/canonical.rs @@ -33,7 +33,7 @@ pub(super) fn canonicalize_fsst( views, Arc::from(buffers), array.dtype().clone(), - array.codes().validity(), + array.codes().validity()?, ) .into_array() }) diff --git a/encodings/fsst/src/compute/like.rs b/encodings/fsst/src/compute/like.rs index 2ef7508c60d..45f6a7f9d9d 100644 --- a/encodings/fsst/src/compute/like.rs +++ b/encodings/fsst/src/compute/like.rs @@ -72,7 +72,7 @@ impl LikeKernel for FSST { // directly without cloning the entire FSSTArray into an ArrayRef. let validity = array .codes() - .validity() + .validity()? .union_nullability(pattern_scalar.dtype().nullability()); Ok(Some(BoolArray::new(result, validity).into_array())) diff --git a/encodings/parquet-variant/src/array.rs b/encodings/parquet-variant/src/array.rs index df872889391..932274d77f8 100644 --- a/encodings/parquet-variant/src/array.rs +++ b/encodings/parquet-variant/src/array.rs @@ -17,7 +17,9 @@ use vortex_array::arrow::ArrowArrayExecutor; use vortex_array::arrow::FromArrowArray; use vortex_array::arrow::to_arrow_null_buffer; use vortex_array::dtype::DType; +use vortex_array::dtype::Nullability; use vortex_array::validity::Validity; +use vortex_array::vtable::child_to_validity; use vortex_array::vtable::validity_to_child; use vortex_buffer::BitBuffer; use vortex_error::VortexExpect; @@ -66,7 +68,7 @@ pub(crate) const SLOT_NAMES: [&str; NUM_SLOTS] = ["validity", "metadata", "value /// (which includes nullability). #[derive(Clone, Debug)] pub struct ParquetVariantData { - pub(crate) validity: Validity, + pub(crate) nullability: Nullability, pub(crate) slots: Vec>, } @@ -108,12 +110,16 @@ impl ParquetVariantData { let validity_child = validity_to_child(&validity, len); let slots = vec![validity_child, Some(metadata), value, typed_value]; - Ok(Self { validity, slots }) + Ok(Self { + nullability: validity.nullability(), + slots, + }) } pub(crate) fn validate(&self, dtype: &DType, len: usize) -> VortexResult<()> { + let validity = self.validity(); Self::validate_parts( - &self.validity, + &validity, self.metadata_array(), self.value_array(), self.typed_value_array(), @@ -146,7 +152,7 @@ impl ParquetVariantData { ); vortex_ensure_eq!( metadata.dtype(), - &DType::Binary(vortex_array::dtype::Nullability::NonNullable), + &DType::Binary(Nullability::NonNullable), "metadata dtype must be non-nullable binary" ); vortex_ensure_eq!( @@ -179,6 +185,10 @@ impl ParquetVariantData { .vortex_expect("ParquetVariantArray metadata slot") } + pub fn validity(&self) -> Validity { + child_to_validity(&self.slots[VALIDITY_SLOT], self.nullability) + } + /// Returns a reference to the un-shredded value child array, if present. pub fn value_array(&self) -> Option<&ArrayRef> { self.slots[VALUE_SLOT].as_ref() @@ -236,7 +246,7 @@ impl ParquetVariantData { pub fn to_arrow(&self, ctx: &mut ExecutionCtx) -> VortexResult { let metadata = self.metadata_array(); let len = metadata.len(); - let nulls = to_arrow_null_buffer(self.validity.clone(), len, ctx)?; + let nulls = to_arrow_null_buffer(self.validity(), len, ctx)?; let mut fields = Vec::with_capacity(3); let mut arrays: Vec = Vec::with_capacity(3); diff --git a/encodings/parquet-variant/src/kernel.rs b/encodings/parquet-variant/src/kernel.rs index 69e922e1548..6061157c432 100644 --- a/encodings/parquet-variant/src/kernel.rs +++ b/encodings/parquet-variant/src/kernel.rs @@ -31,7 +31,7 @@ impl SliceKernel for ParquetVariant { range: Range, _ctx: &mut ExecutionCtx, ) -> VortexResult> { - let validity = array.validity.slice(range.clone())?; + let validity = array.validity()?.slice(range.clone())?; let metadata = array.metadata_array().slice(range.clone())?; let value = array .value_array() @@ -53,7 +53,7 @@ impl FilterKernel for ParquetVariant { mask: &Mask, _ctx: &mut ExecutionCtx, ) -> VortexResult> { - let validity = array.validity.filter(mask)?; + let validity = array.validity()?.filter(mask)?; let metadata = array.metadata_array().filter(mask.clone())?; let value = array .value_array() @@ -75,7 +75,7 @@ impl TakeExecute for ParquetVariant { indices: &ArrayRef, _ctx: &mut ExecutionCtx, ) -> VortexResult> { - let validity = array.validity.take(indices)?; + let validity = array.validity()?.take(indices)?; let metadata = array.metadata_array().take(indices.clone())?; let value = array .value_array() diff --git a/encodings/parquet-variant/src/operations.rs b/encodings/parquet-variant/src/operations.rs index 4b2d26b77a4..67b6c78016b 100644 --- a/encodings/parquet-variant/src/operations.rs +++ b/encodings/parquet-variant/src/operations.rs @@ -42,7 +42,7 @@ impl OperationsVTable for ParquetVariant { index: usize, _ctx: &mut ExecutionCtx, ) -> VortexResult { - if array.validity.is_null(index)? { + if array.validity()?.is_null(index)? { return Ok(Scalar::null(DType::Variant(Nullability::Nullable))); } diff --git a/encodings/parquet-variant/src/validity.rs b/encodings/parquet-variant/src/validity.rs index 3f7b33c6eba..2dde87a18f1 100644 --- a/encodings/parquet-variant/src/validity.rs +++ b/encodings/parquet-variant/src/validity.rs @@ -1,13 +1,16 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use vortex_error::VortexResult; + +use vortex_array::ArrayView; use vortex_array::validity::Validity; -use vortex_array::vtable::ValidityHelper; +use vortex_array::vtable::ValidityVTable; -use crate::array::ParquetVariantData; +use crate::vtable::ParquetVariant; -impl ValidityHelper for ParquetVariantData { - fn validity(&self) -> &Validity { - &self.validity +impl ValidityVTable for ParquetVariant { + fn validity(array: ArrayView<'_, ParquetVariant>) -> VortexResult { + Ok(array.data().validity()) } } diff --git a/encodings/parquet-variant/src/vtable.rs b/encodings/parquet-variant/src/vtable.rs index 083cfc42261..370a5cfbda0 100644 --- a/encodings/parquet-variant/src/vtable.rs +++ b/encodings/parquet-variant/src/vtable.rs @@ -23,7 +23,6 @@ use vortex_array::serde::ArrayChildren; use vortex_array::validity::Validity; use vortex_array::vtable; use vortex_array::vtable::VTable; -use vortex_array::vtable::ValidityVTableFromValidityHelper; use vortex_error::VortexResult; use vortex_error::vortex_ensure; use vortex_error::vortex_err; @@ -34,7 +33,6 @@ use vortex_session::VortexSession; use crate::array::NUM_SLOTS; use crate::array::ParquetVariantData; use crate::array::SLOT_NAMES; -use crate::array::VALIDITY_SLOT; use crate::kernel::PARENT_KERNELS; /// VTable for [`ParquetVariantArray`]. @@ -63,7 +61,7 @@ vtable!(ParquetVariant, ParquetVariant, ParquetVariantData); impl VTable for ParquetVariant { type ArrayData = ParquetVariantData; type OperationsVTable = Self; - type ValidityVTable = ValidityVTableFromValidityHelper; + type ValidityVTable = Self; fn id(&self) -> ArrayId { Self::ID @@ -74,7 +72,7 @@ impl VTable for ParquetVariant { } fn array_hash(array: &ParquetVariantData, state: &mut H, precision: Precision) { - array.validity.array_hash(state, precision); + array.validity().array_hash(state, precision); array.metadata_array().array_hash(state, precision); // Hash discriminators so that (value=Some, typed_value=None) and // (value=None, typed_value=Some) produce different hashes. @@ -93,7 +91,7 @@ impl VTable for ParquetVariant { other: &ParquetVariantData, precision: Precision, ) -> bool { - if !array.validity.array_eq(&other.validity, precision) + if !array.validity().array_eq(&other.validity(), precision) || !array .metadata_array() .array_eq(other.metadata_array(), precision) @@ -225,10 +223,6 @@ impl VTable for ParquetVariant { NUM_SLOTS, slots.len() ); - // Update validity from the validity slot. - if let Some(validity_child) = &slots[VALIDITY_SLOT] { - array.validity = Validity::Array(validity_child.clone()); - } array.slots = slots; Ok(()) } diff --git a/encodings/pco/src/array.rs b/encodings/pco/src/array.rs index 265566ec35e..f1e352542d3 100644 --- a/encodings/pco/src/array.rs +++ b/encodings/pco/src/array.rs @@ -469,7 +469,7 @@ impl PcoData { parray.dtype().as_ptype(), metadata, parray.len(), - parray.validity(), + parray.validity()?, )) } diff --git a/encodings/runend/src/arrow.rs b/encodings/runend/src/arrow.rs index 8d9e674f778..25c1ef197f7 100644 --- a/encodings/runend/src/arrow.rs +++ b/encodings/runend/src/arrow.rs @@ -33,7 +33,7 @@ where let ends_array = PrimitiveArray::from_buffer_handle( ends.buffer_handle().clone(), ends.ptype(), - ends.validity(), + ends.validity()?, ) .into_array(); let (ends_slice, values_slice) = if offset == 0 && len == array.run_ends().max_value() { diff --git a/encodings/runend/src/compress.rs b/encodings/runend/src/compress.rs index 04dad945531..b36b549225c 100644 --- a/encodings/runend/src/compress.rs +++ b/encodings/runend/src/compress.rs @@ -33,7 +33,10 @@ use crate::iter::trimmed_ends_iter; /// Run-end encode a `PrimitiveArray`, returning a tuple of `(ends, values)`. pub fn runend_encode(array: ArrayView) -> (PrimitiveArray, ArrayRef) { - let validity = match array.validity() { + let validity = match array + .validity() + .vortex_expect("run-end validity should be derivable") + { Validity::NonNullable => None, Validity::AllValid => None, Validity::AllInvalid => { diff --git a/encodings/runend/src/compute/take.rs b/encodings/runend/src/compute/take.rs index e2104499621..511b473c53b 100644 --- a/encodings/runend/src/compute/take.rs +++ b/encodings/runend/src/compute/take.rs @@ -49,7 +49,8 @@ impl TakeExecute for RunEnd { .collect::>>()? }); - take_indices_unchecked(&array, &checked_indices, &primitive_indices.validity()).map(Some) + let indices_validity = primitive_indices.validity()?; + take_indices_unchecked(&array, &checked_indices, &indices_validity).map(Some) } } diff --git a/encodings/sparse/src/canonical.rs b/encodings/sparse/src/canonical.rs index c0da00a14d7..7afe3ce2b94 100644 --- a/encodings/sparse/src/canonical.rs +++ b/encodings/sparse/src/canonical.rs @@ -283,6 +283,7 @@ fn execute_sparse_fixed_size_list_inner( // Append the patch value, handling null patches by appending defaults. if values .validity() + .vortex_expect("sparse fixed-size-list validity should be derivable") .is_valid(patch_idx) .vortex_expect("is_valid") { diff --git a/encodings/zigzag/src/compress.rs b/encodings/zigzag/src/compress.rs index b2d142c755c..998a39f2b49 100644 --- a/encodings/zigzag/src/compress.rs +++ b/encodings/zigzag/src/compress.rs @@ -7,6 +7,7 @@ use vortex_array::dtype::NativePType; use vortex_array::dtype::PType; use vortex_array::validity::Validity; use vortex_buffer::BufferMut; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; use vortex_error::vortex_panic; @@ -15,7 +16,7 @@ use zigzag::ZigZag as ExternalZigZag; use crate::ZigZag; use crate::ZigZagArray; pub fn zigzag_encode(parray: PrimitiveArray) -> VortexResult { - let validity = parray.validity(); + let validity = parray.validity()?; let encoded = match parray.ptype() { PType::I8 => zigzag_encode_primitive::(parray.into_buffer_mut(), validity), PType::I16 => zigzag_encode_primitive::(parray.into_buffer_mut(), validity), @@ -43,7 +44,9 @@ where } pub fn zigzag_decode(parray: PrimitiveArray) -> PrimitiveArray { - let validity = parray.validity(); + let validity = parray + .validity() + .vortex_expect("zigzag validity should be derivable"); match parray.ptype() { PType::U8 => zigzag_decode_primitive::(parray.into_buffer_mut(), validity), PType::U16 => zigzag_decode_primitive::(parray.into_buffer_mut(), validity), diff --git a/encodings/zstd/src/array.rs b/encodings/zstd/src/array.rs index a494cfc572f..48e25ec1512 100644 --- a/encodings/zstd/src/array.rs +++ b/encodings/zstd/src/array.rs @@ -684,7 +684,7 @@ impl ZstdData { frames, metadata, parray.len(), - parray.validity(), + parray.validity()?, )) } @@ -772,7 +772,7 @@ impl ZstdData { frames, metadata, vbv.len(), - vbv.validity(), + vbv.validity()?, )) } diff --git a/fuzz/src/array/fill_null.rs b/fuzz/src/array/fill_null.rs index 1f15f3b6d77..3c47cb06542 100644 --- a/fuzz/src/array/fill_null.rs +++ b/fuzz/src/array/fill_null.rs @@ -57,7 +57,10 @@ fn fill_bool_array( .value() .vortex_expect("cannot have null fill value"); - match array.validity() { + match array + .validity() + .vortex_expect("bool validity should be derivable in fuzz baseline") + { Validity::NonNullable | Validity::AllValid => { BoolArray::new(array.into_bit_buffer(), result_nullability.into()).into_array() } @@ -97,7 +100,10 @@ fn fill_primitive_array( let fill_val = T::try_from(fill_value) .vortex_expect("fill value conversion should succeed in fuzz test"); - match array.validity() { + match array + .validity() + .vortex_expect("primitive validity should be derivable in fuzz baseline") + { Validity::NonNullable | Validity::AllValid => { PrimitiveArray::new(array.to_buffer::(), result_nullability.into()).into_array() } @@ -137,7 +143,10 @@ fn fill_decimal_array( let fill_val = D::try_from(decimal_scalar) .vortex_expect("decimal fill value conversion should succeed in fuzz test"); - match array.validity() { + match array + .validity() + .vortex_expect("decimal validity should be derivable in fuzz baseline") + { Validity::NonNullable | Validity::AllValid => DecimalArray::new( array.buffer::(), decimal_dtype, @@ -175,7 +184,10 @@ fn fill_varbinview_array( result_nullability: Nullability, ) -> ArrayRef { let array_ref = array.clone().into_array(); - match array.validity() { + match array + .validity() + .vortex_expect("varbinview validity should be derivable in fuzz baseline") + { Validity::NonNullable | Validity::AllValid => array.into_array(), Validity::AllInvalid => ConstantArray::new(fill_value.clone(), array.len()).into_array(), Validity::Array(validity_array) => { diff --git a/fuzz/src/array/mask.rs b/fuzz/src/array/mask.rs index 89192b3e360..1155db60926 100644 --- a/fuzz/src/array/mask.rs +++ b/fuzz/src/array/mask.rs @@ -58,11 +58,11 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); BoolArray::new(array.to_bit_buffer(), new_validity).into_array() } Canonical::Primitive(array) => { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); PrimitiveArray::from_buffer_handle( array.buffer_handle().clone(), array.ptype(), @@ -71,14 +71,14 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); match_each_decimal_value_type!(array.values_type(), |D| { DecimalArray::new(array.buffer::(), array.decimal_dtype(), new_validity) .into_array() }) } Canonical::VarBinView(array) => { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); VarBinViewArray::new_handle( array.views_handle().clone(), array.data_buffers().clone(), @@ -88,7 +88,7 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); // SAFETY: Since we are only masking the validity and everything else comes from an // already valid `ListViewArray`, all of the invariants are still upheld. @@ -104,7 +104,7 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); FixedSizeListArray::new( array.elements().clone(), array.list_size(), @@ -114,7 +114,7 @@ pub fn mask_canonical_array(canonical: Canonical, mask: &Mask) -> VortexResult { - let new_validity = mask_validity(&array.validity(), mask); + let new_validity = mask_validity(&array.validity()?, mask); StructArray::try_new_with_dtype( array.unmasked_fields(), array.struct_fields(), diff --git a/vortex-array/src/aggregate_fn/accumulator_grouped.rs b/vortex-array/src/aggregate_fn/accumulator_grouped.rs index 053ca607ea1..46182d1cc2a 100644 --- a/vortex-array/src/aggregate_fn/accumulator_grouped.rs +++ b/vortex-array/src/aggregate_fn/accumulator_grouped.rs @@ -159,6 +159,7 @@ impl GroupedAccumulator { ctx: &mut ExecutionCtx, ) -> VortexResult<()> { let mut elements = groups.elements().clone(); + let groups_validity = groups.validity()?; let session = ctx.session().clone(); let kernels = &session.aggregate_fns().grouped_kernels; @@ -178,7 +179,7 @@ impl GroupedAccumulator { elements.clone(), groups.offsets().clone(), groups.sizes().clone(), - groups.validity(), + groups_validity.clone(), ) }; kernel @@ -198,7 +199,7 @@ impl GroupedAccumulator { let elements = elements.execute::(ctx)?.into_array(); let offsets = groups.offsets(); let sizes = groups.sizes().cast(offsets.dtype().clone())?; - let validity = groups.validity().execute_mask(offsets.len(), ctx)?; + let validity = groups_validity.execute_mask(offsets.len(), ctx)?; match_each_integer_ptype!(offsets.dtype().as_ptype(), |O| { let offsets = offsets.clone().execute::>(ctx)?; @@ -250,6 +251,7 @@ impl GroupedAccumulator { ctx: &mut ExecutionCtx, ) -> VortexResult<()> { let mut elements = groups.elements().clone(); + let groups_validity = groups.validity()?; let session = ctx.session().clone(); let kernels = &session.aggregate_fns().grouped_kernels; @@ -268,7 +270,7 @@ impl GroupedAccumulator { FixedSizeListArray::new_unchecked( elements.clone(), groups.list_size(), - groups.validity(), + groups_validity.clone(), groups.len(), ) }; @@ -288,7 +290,7 @@ impl GroupedAccumulator { // Otherwise, we iterate the offsets and sizes and accumulate each group one by one. let elements = elements.execute::(ctx)?.into_array(); - let validity = groups.validity().execute_mask(groups.len(), ctx)?; + let validity = groups_validity.execute_mask(groups.len(), ctx)?; let mut accumulator = Accumulator::try_new( self.vtable.clone(), diff --git a/vortex-array/src/array/typed.rs b/vortex-array/src/array/typed.rs index 7918c6b9956..93e764e7d3a 100644 --- a/vortex-array/src/array/typed.rs +++ b/vortex-array/src/array/typed.rs @@ -16,7 +16,6 @@ use vortex_error::VortexResult; use crate::ArrayRef; use crate::IntoArray; -use crate::ValidityHelper; use crate::array::ArrayId; use crate::array::ArrayView; use crate::array::VTable; @@ -294,17 +293,6 @@ impl Array { } } -impl Array -where - V::ArrayData: ValidityHelper, -{ - /// Returns a reference to the validity. - #[allow(clippy::same_name_method)] - pub fn validity(&self) -> &Validity { - ValidityHelper::validity(self.data()) - } -} - /// Public API methods that shadow `DynArray` / `ArrayRef` methods. impl Array { #[allow(clippy::same_name_method)] @@ -327,6 +315,11 @@ impl Array { self.inner.take(indices) } + #[allow(clippy::same_name_method)] + pub fn validity(&self) -> VortexResult { + self.inner.validity() + } + #[allow(clippy::same_name_method)] pub fn is_valid(&self, index: usize) -> VortexResult { self.inner.is_valid(index) diff --git a/vortex-array/src/array/view.rs b/vortex-array/src/array/view.rs index e879e2c98dd..7a2027591d9 100644 --- a/vortex-array/src/array/view.rs +++ b/vortex-array/src/array/view.rs @@ -5,12 +5,15 @@ use std::fmt::Debug; use std::fmt::Formatter; use std::ops::Deref; +use vortex_error::VortexResult; + use crate::ArrayRef; use crate::array::Array; use crate::array::ArrayId; use crate::array::VTable; use crate::dtype::DType; use crate::stats::StatsSetRef; +use crate::validity::Validity; /// A lightweight, `Copy`-able typed view into an [`ArrayRef`]. pub struct ArrayView<'a, V: VTable> { @@ -62,23 +65,16 @@ impl<'a, V: VTable> ArrayView<'a, V> { self.array.statistics() } + pub fn validity(&self) -> VortexResult { + self.array.validity() + } + pub fn into_owned(self) -> Array { // SAFETY: we are ourselves type checked as 'V' unsafe { Array::::from_array_ref_unchecked(self.array.clone()) } } } -impl<'a, V: VTable> ArrayView<'a, V> -where - V::ArrayData: crate::vtable::ValidityHelper, -{ - /// Returns a reference to the validity. - #[allow(clippy::same_name_method)] - pub fn validity(&self) -> &'a crate::validity::Validity { - crate::vtable::ValidityHelper::validity(self.data) - } -} - impl AsRef for ArrayView<'_, V> { fn as_ref(&self) -> &ArrayRef { self.array diff --git a/vortex-array/src/array/vtable/validity.rs b/vortex-array/src/array/vtable/validity.rs index ab707c550f9..3ca11cdca9d 100644 --- a/vortex-array/src/array/vtable/validity.rs +++ b/vortex-array/src/array/vtable/validity.rs @@ -9,7 +9,7 @@ use crate::array::VTable; use crate::validity::Validity; pub trait ValidityVTable { - /// Returns the [`Validity`] of the array. +/// Returns the [`Validity`] of the array. /// /// ## Pre-conditions /// @@ -17,23 +17,6 @@ pub trait ValidityVTable { fn validity(array: ArrayView<'_, V>) -> VortexResult; } -/// An implementation of the [`ValidityVTable`] for arrays that hold validity as a child array. -pub struct ValidityVTableFromValidityHelper; - -/// Expose validity held as a child array. -pub trait ValidityHelper { - fn validity(&self) -> &Validity; -} - -impl ValidityVTable for ValidityVTableFromValidityHelper -where - V::ArrayData: ValidityHelper, -{ - fn validity(array: ArrayView<'_, V>) -> VortexResult { - Ok(array.data().validity().clone()) - } -} - /// An implementation of the [`ValidityVTable`] for arrays that hold an unsliced validity /// and a slice into it. pub struct ValidityVTableFromValiditySliceHelper; diff --git a/vortex-array/src/arrays/bool/array.rs b/vortex-array/src/arrays/bool/array.rs index 688e5301c43..e66deb32cc1 100644 --- a/vortex-array/src/arrays/bool/array.rs +++ b/vortex-array/src/arrays/bool/array.rs @@ -393,7 +393,7 @@ mod tests { fn test_all_some_iter() { let arr = BoolArray::from_iter([Some(true), Some(false)]); - assert!(matches!(arr.validity(), Validity::AllValid)); + assert!(matches!(arr.validity(), Ok(Validity::AllValid))); let scalar = bool::try_from(&arr.scalar_at(0).unwrap()).unwrap(); assert!(scalar); diff --git a/vortex-array/src/arrays/bool/compute/cast.rs b/vortex-array/src/arrays/bool/compute/cast.rs index a00457f1b6b..bff33756255 100644 --- a/vortex-array/src/arrays/bool/compute/cast.rs +++ b/vortex-array/src/arrays/bool/compute/cast.rs @@ -19,7 +19,7 @@ impl CastReduce for Bool { let new_nullability = dtype.nullability(); let new_validity = array - .validity() + .validity()? .cast_nullability(new_nullability, array.len())?; Ok(Some( BoolArray::new(array.to_bit_buffer(), new_validity).into_array(), diff --git a/vortex-array/src/arrays/bool/compute/fill_null.rs b/vortex-array/src/arrays/bool/compute/fill_null.rs index 32bf557d10b..d1a54b045c7 100644 --- a/vortex-array/src/arrays/bool/compute/fill_null.rs +++ b/vortex-array/src/arrays/bool/compute/fill_null.rs @@ -25,7 +25,7 @@ impl FillNullKernel for Bool { .value() .ok_or_else(|| vortex_err!("Fill value must be non null"))?; - Ok(Some(match array.validity() { + Ok(Some(match array.validity()? { Validity::Array(v) => { let v_bool = v.execute::(ctx)?; let bool_buffer = if fill { diff --git a/vortex-array/src/arrays/bool/compute/filter.rs b/vortex-array/src/arrays/bool/compute/filter.rs index 808672e4a37..d4c4d47f862 100644 --- a/vortex-array/src/arrays/bool/compute/filter.rs +++ b/vortex-array/src/arrays/bool/compute/filter.rs @@ -21,7 +21,7 @@ const FILTER_SLICES_DENSITY_THRESHOLD: f64 = 0.8; impl FilterReduce for Bool { fn filter(array: ArrayView<'_, Bool>, mask: &Mask) -> VortexResult> { - let validity = array.validity().filter(mask)?; + let validity = array.validity()?.filter(mask)?; let mask_values = mask .values() diff --git a/vortex-array/src/arrays/bool/compute/mask.rs b/vortex-array/src/arrays/bool/compute/mask.rs index 9fc07001b8c..f55574cab94 100644 --- a/vortex-array/src/arrays/bool/compute/mask.rs +++ b/vortex-array/src/arrays/bool/compute/mask.rs @@ -16,7 +16,7 @@ impl MaskReduce for Bool { Ok(Some( BoolArray::new( array.to_bit_buffer(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) .into_array(), )) diff --git a/vortex-array/src/arrays/bool/compute/rules.rs b/vortex-array/src/arrays/bool/compute/rules.rs index 8fa5d7a2a7f..d129fd13ffe 100644 --- a/vortex-array/src/arrays/bool/compute/rules.rs +++ b/vortex-array/src/arrays/bool/compute/rules.rs @@ -49,7 +49,7 @@ impl ArrayParentReduceRule for BoolMaskedValidityRule { Ok(Some( BoolArray::new( array.to_bit_buffer(), - array.validity().and(parent.validity())?, + array.validity()?.and(parent.validity()?)?, ) .into_array(), )) diff --git a/vortex-array/src/arrays/bool/compute/slice.rs b/vortex-array/src/arrays/bool/compute/slice.rs index 93042c0a4fa..eea4d51b19b 100644 --- a/vortex-array/src/arrays/bool/compute/slice.rs +++ b/vortex-array/src/arrays/bool/compute/slice.rs @@ -17,7 +17,7 @@ impl SliceReduce for Bool { Ok(Some( BoolArray::new( array.to_bit_buffer().slice(range.clone()), - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .into_array(), )) diff --git a/vortex-array/src/arrays/bool/compute/take.rs b/vortex-array/src/arrays/bool/compute/take.rs index 05e1d6f8f68..f585a71332d 100644 --- a/vortex-array/src/arrays/bool/compute/take.rs +++ b/vortex-array/src/arrays/bool/compute/take.rs @@ -45,7 +45,7 @@ impl TakeExecute for Bool { }); Ok(Some( - BoolArray::new(buffer, array.validity().take(indices)?).into_array(), + BoolArray::new(buffer, array.validity()?.take(indices)?).into_array(), )) } } diff --git a/vortex-array/src/arrays/bool/patch.rs b/vortex-array/src/arrays/bool/patch.rs index 98528c0cbca..87306df7e1e 100644 --- a/vortex-array/src/arrays/bool/patch.rs +++ b/vortex-array/src/arrays/bool/patch.rs @@ -19,8 +19,8 @@ impl BoolArray { let values = patches.values().clone().execute::(ctx)?; let patched_validity = - self.validity() - .patch(len, offset, patches.indices(), &values.validity(), ctx)?; + self.validity()? + .patch(len, offset, patches.indices(), &values.validity()?, ctx)?; let bit_buffer = self.into_bit_buffer(); let mut own_values = bit_buffer diff --git a/vortex-array/src/arrays/constant/vtable/canonical.rs b/vortex-array/src/arrays/constant/vtable/canonical.rs index a257a4c9b8d..5ca796da366 100644 --- a/vortex-array/src/arrays/constant/vtable/canonical.rs +++ b/vortex-array/src/arrays/constant/vtable/canonical.rs @@ -319,6 +319,7 @@ mod tests { use enum_iterator::all; use itertools::Itertools; + use vortex_error::VortexExpect; use vortex_error::VortexResult; use crate::IntoArray; @@ -501,7 +502,7 @@ mod tests { assert_eq!(canonical.len(), 4); assert_eq!(canonical.list_size(), 3); - assert!(matches!(canonical.validity(), Validity::NonNullable)); + assert!(matches!(canonical.validity(), Ok(Validity::NonNullable))); // Check that each list is [10, 20, 30]. for i in 0..4 { @@ -528,7 +529,7 @@ mod tests { assert_eq!(canonical.len(), 3); assert_eq!(canonical.list_size(), 2); - assert!(matches!(canonical.validity(), Validity::AllValid)); + assert!(matches!(canonical.validity(), Ok(Validity::AllValid))); // Check elements. let elements = canonical.elements().to_primitive(); @@ -552,7 +553,7 @@ mod tests { assert_eq!(canonical.len(), 5); assert_eq!(canonical.list_size(), 4); - assert!(matches!(canonical.validity(), Validity::AllInvalid)); + assert!(matches!(canonical.validity(), Ok(Validity::AllInvalid))); // Elements should be defaults (zeros). let elements = canonical.elements().to_primitive(); @@ -574,7 +575,7 @@ mod tests { assert_eq!(canonical.len(), 10); assert_eq!(canonical.list_size(), 0); - assert!(matches!(canonical.validity(), Validity::NonNullable)); + assert!(matches!(canonical.validity(), Ok(Validity::NonNullable))); // Elements array should be empty. assert!(canonical.elements().is_empty()); @@ -640,7 +641,7 @@ mod tests { assert_eq!(canonical.len(), 3); assert_eq!(canonical.list_size(), 3); - assert!(matches!(canonical.validity(), Validity::NonNullable)); + assert!(matches!(canonical.validity(), Ok(Validity::NonNullable))); // Check elements including nulls. let elements = canonical.elements().to_primitive(); @@ -652,7 +653,9 @@ mod tests { assert_eq!(elements.scalar_at(2).unwrap(), Scalar::from(200i32)); // Check element validity. - let element_validity = elements.validity(); + let element_validity = elements + .validity() + .vortex_expect("constant canonical element validity should be derivable"); assert!(element_validity.is_valid(0).unwrap()); assert!(!element_validity.is_valid(1).unwrap()); assert!(element_validity.is_valid(2).unwrap()); diff --git a/vortex-array/src/arrays/datetime/test.rs b/vortex-array/src/arrays/datetime/test.rs index 8bf0b3d8989..3d270ec7053 100644 --- a/vortex-array/src/arrays/datetime/test.rs +++ b/vortex-array/src/arrays/datetime/test.rs @@ -3,6 +3,7 @@ use rstest::rstest; use vortex_buffer::buffer; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use crate::IntoArray; @@ -199,6 +200,7 @@ fn test_validity_preservation(#[case] validity: Validity) { .temporal_values() .to_primitive() .validity() + .vortex_expect("temporal validity should be derivable") .array_eq(&validity, Precision::Ptr) ); } diff --git a/vortex-array/src/arrays/decimal/array.rs b/vortex-array/src/arrays/decimal/array.rs index a51657fd958..df5be8ce202 100644 --- a/vortex-array/src/arrays/decimal/array.rs +++ b/vortex-array/src/arrays/decimal/array.rs @@ -408,7 +408,7 @@ impl DecimalData { let patch_indices = patches.indices().clone().execute::(ctx)?; let patch_values = patches.values().clone().execute::(ctx)?; - let patch_validity = patch_values.validity(); + let patch_validity = patch_values.validity()?; let patched_validity = self.validity().patch( self.len(), offset, diff --git a/vortex-array/src/arrays/decimal/compute/between.rs b/vortex-array/src/arrays/decimal/compute/between.rs index b521d2148e0..3594e5a268c 100644 --- a/vortex-array/src/arrays/decimal/compute/between.rs +++ b/vortex-array/src/arrays/decimal/compute/between.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors use vortex_buffer::BitBuffer; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; @@ -106,7 +107,9 @@ fn between_impl( let value = buffer[idx]; lower_op(lower, value) & upper_op(value, upper) }), - arr.validity().union_nullability(nullability), + arr.validity() + .vortex_expect("validity should be derivable") + .union_nullability(nullability), ) .into_array() } diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index 2ca8b54c944..80377102b7d 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -61,7 +61,7 @@ impl CastKernel for Decimal { // Cast the validity to the new nullability let new_validity = array - .validity() + .validity()? .cast_nullability(*to_nullability, array.len())?; // If the target needs a wider physical type, upcast the values @@ -119,7 +119,7 @@ pub fn upcast_decimal_values( } let decimal_dtype = array.decimal_dtype(); - let validity = array.validity(); + let validity = array.validity()?; // Use match_each_decimal_value_type to dispatch based on source and target types match_each_decimal_value_type!(from_values_type, |F| { @@ -174,7 +174,7 @@ mod tests { .to_decimal(); assert_eq!(casted.dtype(), &nullable_dtype); - assert!(matches!(casted.validity(), Validity::AllValid)); + assert!(matches!(casted.validity(), Ok(Validity::AllValid))); assert_eq!(casted.len(), 3); } @@ -194,7 +194,7 @@ mod tests { .to_decimal(); assert_eq!(casted.dtype(), &non_nullable_dtype); - assert!(matches!(casted.validity(), Validity::NonNullable)); + assert!(matches!(casted.validity(), Ok(Validity::NonNullable))); } #[test] diff --git a/vortex-array/src/arrays/decimal/compute/fill_null.rs b/vortex-array/src/arrays/decimal/compute/fill_null.rs index 7200d35b113..e697a277cca 100644 --- a/vortex-array/src/arrays/decimal/compute/fill_null.rs +++ b/vortex-array/src/arrays/decimal/compute/fill_null.rs @@ -31,7 +31,7 @@ impl FillNullKernel for Decimal { ) -> VortexResult> { let result_validity = Validity::from(fill_value.dtype().nullability()); - Ok(Some(match array.validity() { + Ok(Some(match array.validity()? { Validity::Array(is_valid) => { let is_invalid = is_valid.execute::(ctx)?.into_bit_buffer().not(); let decimal_scalar = fill_value.as_decimal(); diff --git a/vortex-array/src/arrays/decimal/compute/mask.rs b/vortex-array/src/arrays/decimal/compute/mask.rs index aeb39a144fd..804e6557f5c 100644 --- a/vortex-array/src/arrays/decimal/compute/mask.rs +++ b/vortex-array/src/arrays/decimal/compute/mask.rs @@ -22,7 +22,7 @@ impl MaskReduce for Decimal { DecimalArray::new_unchecked( array.buffer::(), array.decimal_dtype(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) } .into_array() diff --git a/vortex-array/src/arrays/decimal/compute/rules.rs b/vortex-array/src/arrays/decimal/compute/rules.rs index 4542749ebb5..fae0c3dd866 100644 --- a/vortex-array/src/arrays/decimal/compute/rules.rs +++ b/vortex-array/src/arrays/decimal/compute/rules.rs @@ -49,7 +49,7 @@ impl ArrayParentReduceRule for DecimalMaskedValidityRule { DecimalArray::new_unchecked( array.buffer::(), array.decimal_dtype(), - array.validity().and(parent.validity())?, + array.validity()?.and(parent.validity()?)?, ) } .into_array() @@ -63,7 +63,7 @@ impl SliceReduce for Decimal { fn slice(array: ArrayView<'_, Self>, range: Range) -> VortexResult> { let result = match_each_decimal_value_type!(array.values_type(), |D| { let sliced = array.buffer::().slice(range.clone()); - let validity = array.validity().slice(range)?; + let validity = array.validity()?.slice(range)?; // SAFETY: Slicing preserves all DecimalArray invariants unsafe { DecimalArray::new_unchecked(sliced, array.decimal_dtype(), validity) } .into_array() diff --git a/vortex-array/src/arrays/decimal/compute/take.rs b/vortex-array/src/arrays/decimal/compute/take.rs index 87a507681b9..444fe313d15 100644 --- a/vortex-array/src/arrays/decimal/compute/take.rs +++ b/vortex-array/src/arrays/decimal/compute/take.rs @@ -24,7 +24,7 @@ impl TakeExecute for Decimal { ctx: &mut ExecutionCtx, ) -> VortexResult> { let indices = indices.clone().execute::(ctx)?; - let validity = array.validity().take(&indices.clone().into_array())?; + let validity = array.validity()?.take(&indices.clone().into_array())?; // TODO(joe): if the true count of take indices validity is low, only take array values with // valid indices. diff --git a/vortex-array/src/arrays/decimal/utils.rs b/vortex-array/src/arrays/decimal/utils.rs index 8cb0f8dd22a..a1eafde0fe3 100644 --- a/vortex-array/src/arrays/decimal/utils.rs +++ b/vortex-array/src/arrays/decimal/utils.rs @@ -28,7 +28,9 @@ macro_rules! try_downcast { .map(|v| <$dst as BigCast>::from(v).vortex_expect("decimal conversion failure")) .collect(), $array.decimal_dtype(), - $array.validity().clone(), + $array + .validity() + .vortex_expect("decimal validity should be derivable"), ); } )* diff --git a/vortex-array/src/arrays/filter/execute/bool.rs b/vortex-array/src/arrays/filter/execute/bool.rs index 78ebfcbebe1..e054ca7ed30 100644 --- a/vortex-array/src/arrays/filter/execute/bool.rs +++ b/vortex-array/src/arrays/filter/execute/bool.rs @@ -3,6 +3,7 @@ use std::sync::Arc; +use vortex_error::VortexExpect; use vortex_mask::MaskValues; use crate::arrays::BoolArray; @@ -10,7 +11,7 @@ use crate::arrays::filter::execute::bitbuffer; use crate::arrays::filter::execute::filter_validity; pub fn filter_bool(array: &BoolArray, mask: &Arc) -> BoolArray { - let validity = array.validity(); + let validity = array.validity().vortex_expect("bool validity should be derivable"); let filtered_validity = filter_validity(validity, mask); let bit_buffer = array.to_bit_buffer(); diff --git a/vortex-array/src/arrays/filter/execute/decimal.rs b/vortex-array/src/arrays/filter/execute/decimal.rs index 93aecee68b1..869e1fabe0c 100644 --- a/vortex-array/src/arrays/filter/execute/decimal.rs +++ b/vortex-array/src/arrays/filter/execute/decimal.rs @@ -3,6 +3,7 @@ use std::sync::Arc; +use vortex_error::VortexExpect; use vortex_mask::MaskValues; use crate::arrays::DecimalArray; @@ -11,7 +12,12 @@ use crate::arrays::filter::execute::filter_validity; use crate::match_each_decimal_value_type; pub fn filter_decimal(array: &DecimalArray, mask: &Arc) -> DecimalArray { - let filtered_validity = filter_validity(array.validity(), mask); + let filtered_validity = filter_validity( + array + .validity() + .vortex_expect("decimal validity should be derivable"), + mask, + ); match_each_decimal_value_type!(array.values_type(), |T| { let filtered_buffer = buffer::filter_buffer(array.buffer::(), mask.as_ref()); diff --git a/vortex-array/src/arrays/filter/execute/fixed_size_list.rs b/vortex-array/src/arrays/filter/execute/fixed_size_list.rs index 0d50f7b6e6a..65482f16005 100644 --- a/vortex-array/src/arrays/filter/execute/fixed_size_list.rs +++ b/vortex-array/src/arrays/filter/execute/fixed_size_list.rs @@ -24,7 +24,12 @@ pub fn filter_fixed_size_list( array: &FixedSizeListArray, selection_mask: &Arc, ) -> FixedSizeListArray { - let filtered_validity = filter_validity(array.validity(), selection_mask); + let filtered_validity = filter_validity( + array + .validity() + .vortex_expect("fixed-size-list validity should be derivable"), + selection_mask, + ); let elements = array.elements(); let new_len = selection_mask.true_count(); diff --git a/vortex-array/src/arrays/filter/execute/listview.rs b/vortex-array/src/arrays/filter/execute/listview.rs index c9c0d7d447e..dac1c9d4928 100644 --- a/vortex-array/src/arrays/filter/execute/listview.rs +++ b/vortex-array/src/arrays/filter/execute/listview.rs @@ -40,7 +40,12 @@ pub fn filter_listview(array: &ListViewArray, selection_mask: &Arc) let offsets = array.offsets(); let sizes = array.sizes(); - let new_validity = filter_validity(array.validity(), selection_mask); + let new_validity = filter_validity( + array + .validity() + .vortex_expect("listview validity should be derivable"), + selection_mask, + ); debug_assert!( new_validity .maybe_len() diff --git a/vortex-array/src/arrays/filter/execute/primitive.rs b/vortex-array/src/arrays/filter/execute/primitive.rs index cc02e2fb616..86ef8ecbbb2 100644 --- a/vortex-array/src/arrays/filter/execute/primitive.rs +++ b/vortex-array/src/arrays/filter/execute/primitive.rs @@ -3,6 +3,7 @@ use std::sync::Arc; +use vortex_error::VortexExpect; use vortex_mask::MaskValues; use crate::arrays::PrimitiveArray; @@ -11,7 +12,9 @@ use crate::arrays::filter::execute::filter_validity; use crate::match_each_native_ptype; pub fn filter_primitive(array: &PrimitiveArray, mask: &Arc) -> PrimitiveArray { - let validity = array.validity(); + let validity = array + .validity() + .vortex_expect("primitive validity should be derivable"); let filtered_validity = filter_validity(validity, mask); match_each_native_ptype!(array.ptype(), |T| { diff --git a/vortex-array/src/arrays/filter/execute/struct_.rs b/vortex-array/src/arrays/filter/execute/struct_.rs index e424dbd2d33..f53d0cda6c0 100644 --- a/vortex-array/src/arrays/filter/execute/struct_.rs +++ b/vortex-array/src/arrays/filter/execute/struct_.rs @@ -12,7 +12,12 @@ use crate::arrays::filter::execute::filter_validity; use crate::arrays::filter::execute::values_to_mask; pub fn filter_struct(array: &StructArray, mask: &Arc) -> StructArray { - let filtered_validity = filter_validity(array.validity(), mask); + let filtered_validity = filter_validity( + array + .validity() + .vortex_expect("struct validity should be derivable"), + mask, + ); let mask_for_filter = values_to_mask(mask); let fields: Vec = array diff --git a/vortex-array/src/arrays/fixed_size_list/compute/cast.rs b/vortex-array/src/arrays/fixed_size_list/compute/cast.rs index 9f006288edf..05a6951ad1d 100644 --- a/vortex-array/src/arrays/fixed_size_list/compute/cast.rs +++ b/vortex-array/src/arrays/fixed_size_list/compute/cast.rs @@ -24,7 +24,7 @@ impl CastReduce for FixedSizeList { let elements = array.elements().cast((**target_element_type).clone())?; let validity = array - .validity() + .validity()? .cast_nullability(dtype.nullability(), array.len())?; Ok(Some( diff --git a/vortex-array/src/arrays/fixed_size_list/compute/mask.rs b/vortex-array/src/arrays/fixed_size_list/compute/mask.rs index 204d5192670..612426a4c59 100644 --- a/vortex-array/src/arrays/fixed_size_list/compute/mask.rs +++ b/vortex-array/src/arrays/fixed_size_list/compute/mask.rs @@ -22,7 +22,7 @@ impl MaskReduce for FixedSizeList { FixedSizeListArray::new_unchecked( array.elements().clone(), array.list_size(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, array.len(), ) } diff --git a/vortex-array/src/arrays/fixed_size_list/compute/slice.rs b/vortex-array/src/arrays/fixed_size_list/compute/slice.rs index 4bbbc5f7fc3..6a26246de76 100644 --- a/vortex-array/src/arrays/fixed_size_list/compute/slice.rs +++ b/vortex-array/src/arrays/fixed_size_list/compute/slice.rs @@ -25,7 +25,7 @@ impl SliceReduce for FixedSizeList { .elements() .slice(range.start * list_size..range.end * list_size)?, array.list_size(), - array.validity().slice(range)?, + array.validity()?.slice(range)?, new_len, ) } diff --git a/vortex-array/src/arrays/fixed_size_list/compute/take.rs b/vortex-array/src/arrays/fixed_size_list/compute/take.rs index 2d1a8c9a37b..ac9090c5a4b 100644 --- a/vortex-array/src/arrays/fixed_size_list/compute/take.rs +++ b/vortex-array/src/arrays/fixed_size_list/compute/take.rs @@ -61,7 +61,7 @@ fn take_with_indices( ); // Since there are no elements to take, we just need to take on the validity map. - let new_validity = array.validity().take(indices)?; + let new_validity = array.validity()?.take(indices)?; let new_len = indices_array.len(); Ok( diff --git a/vortex-array/src/arrays/list/compute/cast.rs b/vortex-array/src/arrays/list/compute/cast.rs index ff8b5ac3731..c2357de99f0 100644 --- a/vortex-array/src/arrays/list/compute/cast.rs +++ b/vortex-array/src/arrays/list/compute/cast.rs @@ -19,7 +19,7 @@ impl CastReduce for List { }; let validity = array - .validity() + .validity()? .cast_nullability(dtype.nullability(), array.len())?; let new_elements = array.elements().cast((**target_element_type).clone())?; diff --git a/vortex-array/src/arrays/list/compute/filter.rs b/vortex-array/src/arrays/list/compute/filter.rs index f09460cbe54..87f56fcb7a8 100644 --- a/vortex-array/src/arrays/list/compute/filter.rs +++ b/vortex-array/src/arrays/list/compute/filter.rs @@ -103,7 +103,7 @@ impl FilterKernel for List { Mask::Values(v) => v, }; - let new_validity = match array.validity() { + let new_validity = match array.validity()? { Validity::NonNullable => Validity::NonNullable, Validity::AllValid => Validity::AllValid, Validity::AllInvalid => { diff --git a/vortex-array/src/arrays/list/compute/mask.rs b/vortex-array/src/arrays/list/compute/mask.rs index bf8b5b028e8..24e6e79935b 100644 --- a/vortex-array/src/arrays/list/compute/mask.rs +++ b/vortex-array/src/arrays/list/compute/mask.rs @@ -16,7 +16,7 @@ impl MaskReduce for List { ListArray::try_new( array.elements().clone(), array.offsets().clone(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) .map(|a| Some(a.into_array())) } diff --git a/vortex-array/src/arrays/list/compute/slice.rs b/vortex-array/src/arrays/list/compute/slice.rs index 97707d6eb02..dc747257f04 100644 --- a/vortex-array/src/arrays/list/compute/slice.rs +++ b/vortex-array/src/arrays/list/compute/slice.rs @@ -18,7 +18,7 @@ impl SliceReduce for List { ListArray::new( array.elements().clone(), array.offsets().slice(range.start..range.end + 1)?, - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .into_array(), )) diff --git a/vortex-array/src/arrays/list/compute/take.rs b/vortex-array/src/arrays/list/compute/take.rs index cb609164e35..cc053c1d588 100644 --- a/vortex-array/src/arrays/list/compute/take.rs +++ b/vortex-array/src/arrays/list/compute/take.rs @@ -109,7 +109,7 @@ fn _take( Ok(ListArray::try_new( new_elements, new_offsets, - array.validity().take(indices_array.array())?, + array.validity()?.take(indices_array.array())?, )? .into_array()) } @@ -178,7 +178,7 @@ fn _take_nullable for ListViewFilterPushDown { array.elements().clone(), array.offsets().filter(parent.filter_mask().clone())?, array.sizes().filter(parent.filter_mask().clone())?, - array.validity().filter(parent.filter_mask())?, + array.validity()?.filter(parent.filter_mask())?, ) } .into_array(), diff --git a/vortex-array/src/arrays/listview/compute/slice.rs b/vortex-array/src/arrays/listview/compute/slice.rs index 3c42a815fbf..15dd83d8232 100644 --- a/vortex-array/src/arrays/listview/compute/slice.rs +++ b/vortex-array/src/arrays/listview/compute/slice.rs @@ -20,7 +20,7 @@ impl SliceReduce for ListView { array.elements().clone(), array.offsets().slice(range.clone())?, array.sizes().slice(range.clone())?, - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .with_zero_copy_to_list(array.is_zero_copy_to_list()) } diff --git a/vortex-array/src/arrays/listview/compute/take.rs b/vortex-array/src/arrays/listview/compute/take.rs index 804f4dc05d9..9f0d97bd98d 100644 --- a/vortex-array/src/arrays/listview/compute/take.rs +++ b/vortex-array/src/arrays/listview/compute/take.rs @@ -47,7 +47,7 @@ impl TakeReduce for ListView { let sizes = array.sizes(); // Compute the new validity by combining the array's validity with the indices' validity. - let new_validity = array.validity().take(indices)?; + let new_validity = array.validity()?.take(indices)?; // Take the offsets and sizes arrays at the requested indices. // Take can reorder offsets, create gaps, and may introduce overlaps if the `indices` diff --git a/vortex-array/src/arrays/listview/conversion.rs b/vortex-array/src/arrays/listview/conversion.rs index d2334c59526..c6b0633bbcf 100644 --- a/vortex-array/src/arrays/listview/conversion.rs +++ b/vortex-array/src/arrays/listview/conversion.rs @@ -64,7 +64,7 @@ pub fn list_view_from_list(list: ListArray, ctx: &mut ExecutionCtx) -> VortexRes list.elements().clone(), adjusted_offsets, sizes, - list.validity(), + list.validity()?, ) .with_zero_copy_to_list(true) }) @@ -127,7 +127,7 @@ pub fn list_from_list_view(list_view: ListViewArray) -> VortexResult ListArray::new_unchecked( zctl_array.elements().clone(), list_offsets, - zctl_array.validity(), + zctl_array.validity()?, ) }) } @@ -206,7 +206,7 @@ pub fn recursive_list_from_list_view(array: ArrayRef) -> VortexResult converted_elements, listview.offsets().clone(), listview.sizes().clone(), - listview.validity(), + listview.validity()?, ) .with_zero_copy_to_list(listview.is_zero_copy_to_list()) } @@ -227,7 +227,7 @@ pub fn recursive_list_from_list_view(array: ArrayRef) -> VortexResult FixedSizeListArray::try_new( converted_elements, fixed_size_list.list_size(), - fixed_size_list.validity(), + fixed_size_list.validity()?, fixed_size_list.len(), ) .vortex_expect( @@ -255,7 +255,7 @@ pub fn recursive_list_from_list_view(array: ArrayRef) -> VortexResult struct_array.names().clone(), converted_fields, struct_array.len(), - struct_array.validity(), + struct_array.validity()?, ) .vortex_expect("StructArray reconstruction should not fail with valid components") .into_array() @@ -282,6 +282,7 @@ pub fn recursive_list_from_list_view(array: ArrayRef) -> VortexResult mod tests { use vortex_buffer::buffer; + use vortex_error::VortexExpect; use vortex_error::VortexResult; use super::super::tests::common::create_basic_listview; @@ -390,6 +391,7 @@ mod tests { assert!( nullable_list_view .validity() + .vortex_expect("listview validity should be derivable") .array_eq(&validity, Precision::Ptr) ); assert_eq!(nullable_list_view.len(), 3); diff --git a/vortex-array/src/arrays/listview/rebuild.rs b/vortex-array/src/arrays/listview/rebuild.rs index 3333d96fdcf..7b37b82c03a 100644 --- a/vortex-array/src/arrays/listview/rebuild.rs +++ b/vortex-array/src/arrays/listview/rebuild.rs @@ -161,7 +161,7 @@ impl ListViewArray { let mut n_elements = NewOffset::zero(); for index in 0..len { - if !self.validity().is_valid(index)? { + if !self.validity()?.is_valid(index)? { new_offsets.push(n_elements); new_sizes.push(S::zero()); continue; @@ -186,7 +186,7 @@ impl ListViewArray { // non-overlapping, all (offset, size) pairs reference valid elements, and the validity // array is preserved from the original. Ok(unsafe { - ListViewArray::new_unchecked(elements, offsets, sizes, self.validity()) + ListViewArray::new_unchecked(elements, offsets, sizes, self.validity()?) .with_zero_copy_to_list(true) }) } @@ -229,7 +229,7 @@ impl ListViewArray { let mut n_elements = NewOffset::zero(); for index in 0..len { - if !self.validity().is_valid(index)? { + if !self.validity()?.is_valid(index)? { // For NULL lists, place them after the previous item's data to maintain the // no-overlap invariant for zero-copy to `ListArray` arrays. new_offsets.push(n_elements); @@ -269,7 +269,7 @@ impl ListViewArray { // - The array satisfies the zero-copy-to-list property by having sorted offsets, no gaps, // and no overlaps. Ok(unsafe { - ListViewArray::new_unchecked(elements, offsets, sizes, self.validity()) + ListViewArray::new_unchecked(elements, offsets, sizes, self.validity()?) .with_zero_copy_to_list(true) }) } @@ -349,7 +349,7 @@ impl ListViewArray { sliced_elements, adjusted_offsets, self.sizes().clone(), - self.validity(), + self.validity()?, ) .with_zero_copy_to_list(self.is_zero_copy_to_list()) }) @@ -439,9 +439,9 @@ mod tests { // Verify nullability is preserved assert_eq!(flattened.dtype().nullability(), Nullability::Nullable); - assert!(flattened.validity().is_valid(0).unwrap()); - assert!(!flattened.validity().is_valid(1).unwrap()); - assert!(flattened.validity().is_valid(2).unwrap()); + assert!(flattened.validity()?.is_valid(0).unwrap()); + assert!(!flattened.validity()?.is_valid(1).unwrap()); + assert!(flattened.validity()?.is_valid(2).unwrap()); // Verify valid lists contain correct data assert_arrays_eq!( diff --git a/vortex-array/src/arrays/masked/compute/filter.rs b/vortex-array/src/arrays/masked/compute/filter.rs index 0eb8fb25011..9d33ac6767d 100644 --- a/vortex-array/src/arrays/masked/compute/filter.rs +++ b/vortex-array/src/arrays/masked/compute/filter.rs @@ -14,7 +14,7 @@ use crate::arrays::filter::FilterReduce; impl FilterReduce for Masked { fn filter(array: ArrayView<'_, Masked>, mask: &Mask) -> VortexResult> { // Filter the validity to get the new validity - let filtered_validity = array.validity().filter(mask)?; + let filtered_validity = array.validity()?.filter(mask)?; // Filter the child array // The child is guaranteed to have no nulls, so filtering it is straightforward diff --git a/vortex-array/src/arrays/masked/compute/mask.rs b/vortex-array/src/arrays/masked/compute/mask.rs index a092b8f69b0..c80a1a05082 100644 --- a/vortex-array/src/arrays/masked/compute/mask.rs +++ b/vortex-array/src/arrays/masked/compute/mask.rs @@ -17,6 +17,7 @@ impl MaskReduce for Masked { // AND the existing validity mask with the new mask and push into child. let combined_mask = array .validity() + ? .and(Validity::Array(mask.clone()))? .to_array(array.len()); let masked_child = MaskExpr.try_new_array( diff --git a/vortex-array/src/arrays/masked/compute/slice.rs b/vortex-array/src/arrays/masked/compute/slice.rs index 1be55ea6c30..d6c9ee3c033 100644 --- a/vortex-array/src/arrays/masked/compute/slice.rs +++ b/vortex-array/src/arrays/masked/compute/slice.rs @@ -15,7 +15,7 @@ use crate::arrays::slice::SliceReduce; impl SliceReduce for Masked { fn slice(array: ArrayView<'_, Self>, range: Range) -> VortexResult> { let child = array.child().slice(range.clone())?; - let validity = array.validity().slice(range)?; + let validity = array.validity()?.slice(range)?; Ok(Some(MaskedArray::try_new(child, validity)?.into_array())) } diff --git a/vortex-array/src/arrays/masked/compute/take.rs b/vortex-array/src/arrays/masked/compute/take.rs index 7061529b897..c1107c76467 100644 --- a/vortex-array/src/arrays/masked/compute/take.rs +++ b/vortex-array/src/arrays/masked/compute/take.rs @@ -24,7 +24,7 @@ impl TakeReduce for Masked { }; // Compute the new validity by taking from array's validity and merging with indices validity - let taken_validity = array.validity().take(indices)?; + let taken_validity = array.validity()?.take(indices)?; // Construct new MaskedArray Ok(Some( diff --git a/vortex-array/src/arrays/masked/execute.rs b/vortex-array/src/arrays/masked/execute.rs index 0f11a537ba2..b4f8266aabc 100644 --- a/vortex-array/src/arrays/masked/execute.rs +++ b/vortex-array/src/arrays/masked/execute.rs @@ -79,7 +79,7 @@ fn mask_validity_bool( ctx: &mut ExecutionCtx, ) -> VortexResult { let len = array.len(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; Ok(BoolArray::new(array.to_bit_buffer(), new_validity)) } @@ -90,7 +90,7 @@ fn mask_validity_primitive( ) -> VortexResult { let len = array.len(); let ptype = array.ptype(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; // SAFETY: validity has same length as values Ok(unsafe { PrimitiveArray::new_unchecked_from_handle( @@ -109,7 +109,7 @@ fn mask_validity_decimal( let len = array.len(); let dec_dtype = array.decimal_dtype(); let values_type = array.values_type(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; // SAFETY: We're only changing validity, not the data structure Ok(match_each_decimal_value_type!(values_type, |T| { let buffer = array.buffer::(); @@ -125,7 +125,7 @@ fn mask_validity_varbinview( ) -> VortexResult { let len = array.len(); let dtype = array.dtype().as_nullable(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; // SAFETY: We're only changing validity, not the data structure Ok(unsafe { VarBinViewArray::new_handle_unchecked( @@ -143,7 +143,7 @@ fn mask_validity_listview( ctx: &mut ExecutionCtx, ) -> VortexResult { let len = array.len(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; // SAFETY: We're only changing validity, not the data structure Ok(unsafe { ListViewArray::new_unchecked( @@ -162,7 +162,7 @@ fn mask_validity_fixed_size_list( ) -> VortexResult { let len = array.len(); let list_size = array.list_size(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; // SAFETY: We're only changing validity, not the data structure Ok(unsafe { FixedSizeListArray::new_unchecked(array.elements().clone(), list_size, new_validity, len) @@ -175,7 +175,7 @@ fn mask_validity_struct( ctx: &mut ExecutionCtx, ) -> VortexResult { let len = array.len(); - let new_validity = combine_validity(&array.validity(), mask, len, ctx)?; + let new_validity = combine_validity(&array.validity()?, mask, len, ctx)?; let fields = array.unmasked_fields(); let struct_fields = array.struct_fields(); // SAFETY: We're only changing validity, not the data structure diff --git a/vortex-array/src/arrays/masked/tests.rs b/vortex-array/src/arrays/masked/tests.rs index 23acae85e0d..35d533d4f23 100644 --- a/vortex-array/src/arrays/masked/tests.rs +++ b/vortex-array/src/arrays/masked/tests.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors use rstest::rstest; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use super::*; @@ -100,5 +101,11 @@ fn test_masked_child_preserves_length(#[case] validity: Validity) { assert_eq!(array.len(), len); let mut ctx = LEGACY_SESSION.create_execution_ctx(); - assert!(array.validity().mask_eq(&validity, &mut ctx).unwrap(),); + assert!( + array + .validity() + .vortex_expect("masked validity should be derivable") + .mask_eq(&validity, &mut ctx) + .unwrap(), + ); } diff --git a/vortex-array/src/arrays/primitive/array/accessor.rs b/vortex-array/src/arrays/primitive/array/accessor.rs index 26f284eb9e0..c8759d472b2 100644 --- a/vortex-array/src/arrays/primitive/array/accessor.rs +++ b/vortex-array/src/arrays/primitive/array/accessor.rs @@ -3,6 +3,8 @@ use std::iter; +use vortex_error::VortexExpect; + use crate::ToCanonical; use crate::accessor::ArrayAccessor; use crate::arrays::PrimitiveArray; @@ -14,7 +16,7 @@ impl ArrayAccessor for PrimitiveArray { where F: for<'a> FnOnce(&mut dyn Iterator>) -> R, { - match self.validity() { + match self.validity().vortex_expect("primitive validity should be derivable") { Validity::NonNullable | Validity::AllValid => { let mut iter = self.as_slice::().iter().map(Some); f(&mut iter) diff --git a/vortex-array/src/arrays/primitive/array/cast.rs b/vortex-array/src/arrays/primitive/array/cast.rs index f213f6a60c5..65406688afe 100644 --- a/vortex-array/src/arrays/primitive/array/cast.rs +++ b/vortex-array/src/arrays/primitive/array/cast.rs @@ -72,7 +72,12 @@ impl PrimitiveArray { "can't reinterpret cast between integers of two different widths" ); - PrimitiveArray::from_buffer_handle(self.buffer_handle().clone(), ptype, self.validity()) + PrimitiveArray::from_buffer_handle( + self.buffer_handle().clone(), + ptype, + self.validity() + .vortex_expect("primitive validity should be derivable"), + ) } /// Narrow the array to the smallest possible integer type that can represent all values. @@ -85,7 +90,7 @@ impl PrimitiveArray { let Some(min_max) = min_max(&self.clone().into_array(), &mut ctx)? else { return Ok(PrimitiveArray::new( Buffer::::zeroed(self.len()), - self.validity(), + self.validity()?, )); }; @@ -186,7 +191,7 @@ mod tests { result.dtype(), &DType::Primitive(PType::U8, Nullability::Nullable) ); - assert!(matches!(result.validity(), Validity::AllInvalid)); + assert!(matches!(result.validity(), Ok(Validity::AllInvalid))); } #[rstest] @@ -232,7 +237,7 @@ mod tests { &DType::Primitive(PType::U8, Nullability::Nullable) ); // Check that validity is preserved (the array should still have nullable values) - assert!(matches!(&result.validity(), Validity::Array(_))); + assert!(matches!(result.validity(), Ok(Validity::Array(_)))); } #[test] @@ -269,7 +274,7 @@ mod tests { let array2 = PrimitiveArray::new(Buffer::::empty(), Validity::NonNullable); let result2 = array2.narrow().unwrap(); // Empty arrays should not have their validity changed - assert!(matches!(result.validity(), Validity::AllInvalid)); - assert!(matches!(result2.validity(), Validity::NonNullable)); + assert!(matches!(result.validity(), Ok(Validity::AllInvalid))); + assert!(matches!(result2.validity(), Ok(Validity::NonNullable))); } } diff --git a/vortex-array/src/arrays/primitive/array/patch.rs b/vortex-array/src/arrays/primitive/array/patch.rs index 5ea2ba5caa3..8844009d410 100644 --- a/vortex-array/src/arrays/primitive/array/patch.rs +++ b/vortex-array/src/arrays/primitive/array/patch.rs @@ -22,8 +22,8 @@ impl PrimitiveArray { let patch_indices = patches.indices().clone().execute::(ctx)?; let patch_values = patches.values().clone().execute::(ctx)?; - let patch_validity = patch_values.validity(); - let patched_validity = self.validity().patch( + let patch_validity = patch_values.validity()?; + let patched_validity = self.validity()?.patch( self.len(), patches.offset(), &patch_indices.clone().into_array(), diff --git a/vortex-array/src/arrays/primitive/array/top_value.rs b/vortex-array/src/arrays/primitive/array/top_value.rs index 2f325bbd8a7..dae36a5a523 100644 --- a/vortex-array/src/arrays/primitive/array/top_value.rs +++ b/vortex-array/src/arrays/primitive/array/top_value.rs @@ -24,7 +24,7 @@ impl PrimitiveArray { return Ok(None); } - if matches!(self.validity(), Validity::AllInvalid) { + if matches!(self.validity()?, Validity::AllInvalid) { return Ok(None); } diff --git a/vortex-array/src/arrays/primitive/compute/between.rs b/vortex-array/src/arrays/primitive/compute/between.rs index 251aa9c9e7c..9a6c7f60c1b 100644 --- a/vortex-array/src/arrays/primitive/compute/between.rs +++ b/vortex-array/src/arrays/primitive/compute/between.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors use vortex_buffer::BitBuffer; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use crate::ArrayRef; @@ -109,7 +110,9 @@ where let i = unsafe { *slice.get_unchecked(idx) }; lower_fn(lower, i) & upper_fn(i, upper) }), - arr.validity().union_nullability(nullability), + arr.validity() + .vortex_expect("validity should be derivable") + .union_nullability(nullability), ) .into_array() } diff --git a/vortex-array/src/arrays/primitive/compute/cast.rs b/vortex-array/src/arrays/primitive/compute/cast.rs index 28c33117a8a..f2467f6f1ab 100644 --- a/vortex-array/src/arrays/primitive/compute/cast.rs +++ b/vortex-array/src/arrays/primitive/compute/cast.rs @@ -36,7 +36,7 @@ impl CastKernel for Primitive { // First, check that the cast is compatible with the source array's validity let new_validity = array - .validity() + .validity()? .cast_nullability(new_nullability, array.len())?; // Same ptype: zero-copy, just update validity. @@ -156,7 +156,7 @@ mod test { // cast from u32 to u8 let p = arr.cast(PType::U8.into()).unwrap().to_primitive(); assert_arrays_eq!(p, PrimitiveArray::from_iter([0u8, 10, 200])); - assert!(matches!(p.validity(), Validity::NonNullable)); + assert!(matches!(p.validity(), Ok(Validity::NonNullable))); // to nullable let p = p @@ -168,7 +168,7 @@ mod test { p, PrimitiveArray::new(buffer![0u8, 10, 200], Validity::AllValid) ); - assert!(matches!(p.validity(), Validity::AllValid)); + assert!(matches!(p.validity(), Ok(Validity::AllValid))); // back to non-nullable let p = p @@ -177,7 +177,7 @@ mod test { .unwrap() .to_primitive(); assert_arrays_eq!(p, PrimitiveArray::from_iter([0u8, 10, 200])); - assert!(matches!(p.validity(), Validity::NonNullable)); + assert!(matches!(p.validity(), Ok(Validity::NonNullable))); // to nullable u32 let p = p @@ -189,7 +189,7 @@ mod test { p, PrimitiveArray::new(buffer![0u32, 10, 200], Validity::AllValid) ); - assert!(matches!(p.validity(), Validity::AllValid)); + assert!(matches!(p.validity(), Ok(Validity::AllValid))); // to non-nullable u8 let p = p @@ -198,7 +198,7 @@ mod test { .unwrap() .to_primitive(); assert_arrays_eq!(p, PrimitiveArray::from_iter([0u8, 10, 200])); - assert!(matches!(p.validity(), Validity::NonNullable)); + assert!(matches!(p.validity(), Ok(Validity::NonNullable))); } #[test] @@ -294,7 +294,7 @@ mod test { .cast(DType::Primitive(PType::I8, Nullability::Nullable))? .to_primitive(); assert_eq!(casted.len(), 2); - assert!(matches!(casted.validity(), Validity::AllInvalid)); + assert!(matches!(casted.validity(), Ok(Validity::AllInvalid))); Ok(()) } diff --git a/vortex-array/src/arrays/primitive/compute/fill_null.rs b/vortex-array/src/arrays/primitive/compute/fill_null.rs index a24c0133067..d0de9a81a4f 100644 --- a/vortex-array/src/arrays/primitive/compute/fill_null.rs +++ b/vortex-array/src/arrays/primitive/compute/fill_null.rs @@ -26,7 +26,7 @@ impl FillNullKernel for Primitive { ) -> VortexResult> { let result_validity = Validity::from(fill_value.dtype().nullability()); - Ok(Some(match array.validity() { + Ok(Some(match array.validity()? { Validity::Array(is_valid) => { let is_invalid = is_valid.execute::(ctx)?.into_bit_buffer().not(); match_each_native_ptype!(array.ptype(), |T| { diff --git a/vortex-array/src/arrays/primitive/compute/mask.rs b/vortex-array/src/arrays/primitive/compute/mask.rs index d7460b406e3..6768efd6fbf 100644 --- a/vortex-array/src/arrays/primitive/compute/mask.rs +++ b/vortex-array/src/arrays/primitive/compute/mask.rs @@ -18,7 +18,7 @@ impl MaskReduce for Primitive { PrimitiveArray::new_unchecked_from_handle( array.buffer_handle().clone(), array.ptype(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) .into_array() })) diff --git a/vortex-array/src/arrays/primitive/compute/rules.rs b/vortex-array/src/arrays/primitive/compute/rules.rs index 46fb8f62eb2..99b0a7464d5 100644 --- a/vortex-array/src/arrays/primitive/compute/rules.rs +++ b/vortex-array/src/arrays/primitive/compute/rules.rs @@ -38,7 +38,7 @@ impl ArrayParentReduceRule for PrimitiveMaskedValidityRule { ) -> VortexResult> { // TODO(joe): make this lazy // Merge the parent's validity mask into the child's validity - let new_validity = array.validity().and(parent.validity())?; + let new_validity = array.validity()?.and(parent.validity()?)?; // SAFETY: masking validity does not change PrimitiveArray invariants let masked_array = unsafe { diff --git a/vortex-array/src/arrays/primitive/compute/slice.rs b/vortex-array/src/arrays/primitive/compute/slice.rs index b3870ace24d..1830ef3f8c6 100644 --- a/vortex-array/src/arrays/primitive/compute/slice.rs +++ b/vortex-array/src/arrays/primitive/compute/slice.rs @@ -20,7 +20,7 @@ impl SliceReduce for Primitive { PrimitiveArray::from_buffer_handle( array.buffer_handle().slice_typed::(range.clone()), T::PTYPE, - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .into_array() }); diff --git a/vortex-array/src/arrays/primitive/compute/take/mod.rs b/vortex-array/src/arrays/primitive/compute/take/mod.rs index 20cf0f9a355..8afd7dbcf43 100644 --- a/vortex-array/src/arrays/primitive/compute/take/mod.rs +++ b/vortex-array/src/arrays/primitive/compute/take/mod.rs @@ -99,9 +99,7 @@ impl TakeExecute for Primitive { .execute::(ctx)? }; - let validity = array - .validity() - .take(&unsigned_indices.clone().into_array())?; + let validity = array.validity()?.take(&unsigned_indices.clone().into_array())?; // Delegate to the best kernel based on the target CPU { let unsigned_indices = unsigned_indices.as_view(); diff --git a/vortex-array/src/arrays/struct_/array.rs b/vortex-array/src/arrays/struct_/array.rs index 3f75a48e958..86ea3aef1f4 100644 --- a/vortex-array/src/arrays/struct_/array.rs +++ b/vortex-array/src/arrays/struct_/array.rs @@ -629,7 +629,7 @@ impl Array { FieldNames::from(names.as_slice()), children, self.len(), - self.validity(), + self.validity()?, ) } diff --git a/vortex-array/src/arrays/struct_/compute/cast.rs b/vortex-array/src/arrays/struct_/compute/cast.rs index 87c59575e66..5b5bd8abfe0 100644 --- a/vortex-array/src/arrays/struct_/compute/cast.rs +++ b/vortex-array/src/arrays/struct_/compute/cast.rs @@ -71,7 +71,7 @@ impl CastKernel for Struct { } let validity = array - .validity() + .validity()? .cast_nullability(dtype.nullability(), array.len())?; StructArray::try_new( diff --git a/vortex-array/src/arrays/struct_/compute/mask.rs b/vortex-array/src/arrays/struct_/compute/mask.rs index c17493c11bf..a6572d29937 100644 --- a/vortex-array/src/arrays/struct_/compute/mask.rs +++ b/vortex-array/src/arrays/struct_/compute/mask.rs @@ -17,7 +17,7 @@ impl MaskReduce for Struct { array.unmasked_fields().iter().cloned().collect::>(), array.struct_fields(), array.len(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) .map(|a| Some(a.into_array())) } diff --git a/vortex-array/src/arrays/struct_/compute/rules.rs b/vortex-array/src/arrays/struct_/compute/rules.rs index 4d21b190151..1672352afe1 100644 --- a/vortex-array/src/arrays/struct_/compute/rules.rs +++ b/vortex-array/src/arrays/struct_/compute/rules.rs @@ -78,10 +78,10 @@ impl ArrayParentReduceRule for StructCastPushDownRule { } let validity = if parent.options.is_nullable() { - array.validity().into_nullable() + array.validity()?.into_nullable() } else { array - .validity() + .validity()? .into_non_nullable(array.len()) .ok_or_else(|| vortex_err!("Failed to cast nullable struct to non-nullable"))? }; @@ -111,7 +111,7 @@ impl ArrayParentReduceRule for StructGetItemRule { return Ok(None); }; - match child.validity() { + match child.validity()? { Validity::NonNullable | Validity::AllValid => { // If the struct is non-nullable or all valid, the field's validity is unchanged Ok(Some(field.clone())) diff --git a/vortex-array/src/arrays/struct_/compute/slice.rs b/vortex-array/src/arrays/struct_/compute/slice.rs index 0f36d87c560..b5b29a397da 100644 --- a/vortex-array/src/arrays/struct_/compute/slice.rs +++ b/vortex-array/src/arrays/struct_/compute/slice.rs @@ -27,7 +27,7 @@ impl SliceReduce for Struct { fields, array.struct_fields(), range.len(), - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) } .into_array(), diff --git a/vortex-array/src/arrays/struct_/compute/take.rs b/vortex-array/src/arrays/struct_/compute/take.rs index 8ce8b3f3851..817bd4d287f 100644 --- a/vortex-array/src/arrays/struct_/compute/take.rs +++ b/vortex-array/src/arrays/struct_/compute/take.rs @@ -43,7 +43,7 @@ impl TakeReduce for Struct { .collect::, _>>()?, array.struct_fields(), indices.len(), - array.validity().take(indices)?, + array.validity()?.take(indices)?, ) .map(|a| a.into_array()) .map(Some) diff --git a/vortex-array/src/arrays/struct_/compute/zip.rs b/vortex-array/src/arrays/struct_/compute/zip.rs index 75fe4288dd6..6e2615cbef7 100644 --- a/vortex-array/src/arrays/struct_/compute/zip.rs +++ b/vortex-array/src/arrays/struct_/compute/zip.rs @@ -39,8 +39,8 @@ impl ZipKernel for Struct { .map(|(t, f)| ArrayBuiltins::zip(mask, t.clone(), f.clone())) .collect::>>()?; - let v1 = if_true.validity(); - let v2 = if_false.validity(); + let v1 = if_true.validity()?; + let v2 = if_false.validity()?; let validity = match (&v1, &v2) { (Validity::NonNullable, Validity::NonNullable) => Validity::NonNullable, (Validity::AllValid, Validity::AllValid) => Validity::AllValid, diff --git a/vortex-array/src/arrays/varbin/accessor.rs b/vortex-array/src/arrays/varbin/accessor.rs index 990c264e4f3..7421f09fbbe 100644 --- a/vortex-array/src/arrays/varbin/accessor.rs +++ b/vortex-array/src/arrays/varbin/accessor.rs @@ -3,6 +3,8 @@ use std::iter; +use vortex_error::VortexExpect; + use crate::ToCanonical; use crate::accessor::ArrayAccessor; use crate::arrays::VarBinArray; @@ -15,7 +17,9 @@ impl ArrayAccessor<[u8]> for VarBinArray { F: for<'a> FnOnce(&mut dyn Iterator>) -> R, { let offsets = self.offsets().to_primitive(); - let validity = self.validity(); + let validity = self + .validity() + .vortex_expect("varbin validity should be derivable"); let bytes = self.bytes(); let bytes = bytes.as_slice(); diff --git a/vortex-array/src/arrays/varbin/compute/cast.rs b/vortex-array/src/arrays/varbin/compute/cast.rs index e1d7a1b2324..a474168a0a2 100644 --- a/vortex-array/src/arrays/varbin/compute/cast.rs +++ b/vortex-array/src/arrays/varbin/compute/cast.rs @@ -19,7 +19,7 @@ impl CastReduce for VarBin { let new_nullability = dtype.nullability(); let new_validity = array - .validity() + .validity()? .cast_nullability(new_nullability, array.len())?; let new_dtype = array.dtype().with_nullability(new_nullability); Ok(Some( diff --git a/vortex-array/src/arrays/varbin/compute/compare.rs b/vortex-array/src/arrays/varbin/compute/compare.rs index 1c2f8842a75..abce39a4045 100644 --- a/vortex-array/src/arrays/varbin/compute/compare.rs +++ b/vortex-array/src/arrays/varbin/compute/compare.rs @@ -73,7 +73,7 @@ impl CompareKernel for VarBin { return Ok(Some( BoolArray::new( buffer, - lhs.validity().union_nullability(rhs.dtype().nullability()), + lhs.validity()?.union_nullability(rhs.dtype().nullability()), ) .into_array(), )); diff --git a/vortex-array/src/arrays/varbin/compute/filter.rs b/vortex-array/src/arrays/varbin/compute/filter.rs index cfe940e36e9..b1117b35bd9 100644 --- a/vortex-array/src/arrays/varbin/compute/filter.rs +++ b/vortex-array/src/arrays/varbin/compute/filter.rs @@ -166,7 +166,7 @@ fn filter_select_var_bin_by_index( offsets.as_slice::(), values.bytes().as_slice(), mask_indices, - values.validity(), + values.validity()?, selection_count, ) }) diff --git a/vortex-array/src/arrays/varbin/compute/mask.rs b/vortex-array/src/arrays/varbin/compute/mask.rs index 7f26cdf8d1a..8a205acaab5 100644 --- a/vortex-array/src/arrays/varbin/compute/mask.rs +++ b/vortex-array/src/arrays/varbin/compute/mask.rs @@ -18,7 +18,7 @@ impl MaskReduce for VarBin { array.offsets().clone(), array.bytes().clone(), array.dtype().as_nullable(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, )? .into_array(), )) diff --git a/vortex-array/src/arrays/varbin/compute/slice.rs b/vortex-array/src/arrays/varbin/compute/slice.rs index 94f39ebea87..c356c0338d9 100644 --- a/vortex-array/src/arrays/varbin/compute/slice.rs +++ b/vortex-array/src/arrays/varbin/compute/slice.rs @@ -25,7 +25,7 @@ impl VarBin { array.offsets().slice(range.start..range.end + 1)?, array.bytes_handle().clone(), array.dtype().clone(), - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .into_array() }) diff --git a/vortex-array/src/arrays/varbinview/accessor.rs b/vortex-array/src/arrays/varbinview/accessor.rs index 72c98beee5e..ef1470a090d 100644 --- a/vortex-array/src/arrays/varbinview/accessor.rs +++ b/vortex-array/src/arrays/varbinview/accessor.rs @@ -3,6 +3,8 @@ use std::iter; +use vortex_error::VortexExpect; + use crate::ToCanonical; use crate::accessor::ArrayAccessor; use crate::arrays::VarBinViewArray; @@ -19,7 +21,10 @@ impl ArrayAccessor<[u8]> for VarBinViewArray { let views = self.views(); - match self.validity() { + match self + .validity() + .vortex_expect("varbinview validity should be derivable") + { Validity::NonNullable | Validity::AllValid => { let mut iter = views.iter().map(|view| { if view.is_inlined() { diff --git a/vortex-array/src/arrays/varbinview/compute/cast.rs b/vortex-array/src/arrays/varbinview/compute/cast.rs index 7bcfa3af43a..3d5106f0a4a 100644 --- a/vortex-array/src/arrays/varbinview/compute/cast.rs +++ b/vortex-array/src/arrays/varbinview/compute/cast.rs @@ -19,7 +19,7 @@ impl CastReduce for VarBinView { let new_nullability = dtype.nullability(); let new_validity = array - .validity() + .validity()? .cast_nullability(new_nullability, array.len())?; let new_dtype = array.dtype().with_nullability(new_nullability); diff --git a/vortex-array/src/arrays/varbinview/compute/mask.rs b/vortex-array/src/arrays/varbinview/compute/mask.rs index 90655e37533..95630d3fcd5 100644 --- a/vortex-array/src/arrays/varbinview/compute/mask.rs +++ b/vortex-array/src/arrays/varbinview/compute/mask.rs @@ -20,7 +20,7 @@ impl MaskReduce for VarBinView { array.views_handle().clone(), array.data_buffers().clone(), array.dtype().as_nullable(), - array.validity().and(Validity::Array(mask.clone()))?, + array.validity()?.and(Validity::Array(mask.clone()))?, ) .into_array(), )) diff --git a/vortex-array/src/arrays/varbinview/compute/slice.rs b/vortex-array/src/arrays/varbinview/compute/slice.rs index 56121910f1d..19f5a0d41b0 100644 --- a/vortex-array/src/arrays/varbinview/compute/slice.rs +++ b/vortex-array/src/arrays/varbinview/compute/slice.rs @@ -23,7 +23,7 @@ impl SliceReduce for VarBinView { .slice_typed::(range.clone()), Arc::clone(array.data_buffers()), array.dtype().clone(), - array.validity().slice(range)?, + array.validity()?.slice(range)?, ) .into_array(), )) diff --git a/vortex-array/src/arrays/varbinview/compute/take.rs b/vortex-array/src/arrays/varbinview/compute/take.rs index 4b8f930098d..8dfc1dfc4b4 100644 --- a/vortex-array/src/arrays/varbinview/compute/take.rs +++ b/vortex-array/src/arrays/varbinview/compute/take.rs @@ -28,7 +28,7 @@ impl TakeExecute for VarBinView { indices: &ArrayRef, ctx: &mut ExecutionCtx, ) -> VortexResult> { - let validity = array.validity().take(indices)?; + let validity = array.validity()?.take(indices)?; let indices = indices.clone().execute::(ctx)?; let indices_mask = indices.validity_mask()?; diff --git a/vortex-array/src/arrow/executor/byte.rs b/vortex-array/src/arrow/executor/byte.rs index 7fe99d00f27..3e097f03d63 100644 --- a/vortex-array/src/arrow/executor/byte.rs +++ b/vortex-array/src/arrow/executor/byte.rs @@ -67,7 +67,7 @@ where let data = array.bytes().clone().into_arrow_buffer(); - let null_buffer = to_arrow_null_buffer(array.validity(), array.len(), ctx)?; + let null_buffer = to_arrow_null_buffer(array.validity()?, array.len(), ctx)?; Ok(Arc::new(unsafe { GenericByteArray::::new_unchecked(offsets, data, null_buffer) })) diff --git a/vortex-array/src/arrow/executor/byte_view.rs b/vortex-array/src/arrow/executor/byte_view.rs index 8741d1d0ac9..9b19ddcc722 100644 --- a/vortex-array/src/arrow/executor/byte_view.rs +++ b/vortex-array/src/arrow/executor/byte_view.rs @@ -49,7 +49,7 @@ pub fn execute_varbinview_to_arrow( .iter() .map(|buffer| buffer.as_host().clone().into_arrow_buffer()) .collect(); - let nulls = to_arrow_null_buffer(array.validity(), array.len(), ctx)?; + let nulls = to_arrow_null_buffer(array.validity()?, array.len(), ctx)?; // SAFETY: our own VarBinView array is considered safe. Ok(Arc::new(unsafe { diff --git a/vortex-array/src/arrow/executor/fixed_size_list.rs b/vortex-array/src/arrow/executor/fixed_size_list.rs index e82a625a750..ef0d5b53c7c 100644 --- a/vortex-array/src/arrow/executor/fixed_size_list.rs +++ b/vortex-array/src/arrow/executor/fixed_size_list.rs @@ -52,7 +52,7 @@ fn list_to_list( "Cannot convert FixedSizeListArray to non-nullable Arrow array when elements are nullable" ); - let null_buffer = to_arrow_null_buffer(array.validity(), array.len(), ctx)?; + let null_buffer = to_arrow_null_buffer(array.validity()?, array.len(), ctx)?; Ok(Arc::new( arrow_array::FixedSizeListArray::try_new_with_length( diff --git a/vortex-array/src/arrow/executor/list.rs b/vortex-array/src/arrow/executor/list.rs index f17ededa54d..e8742bcf5ab 100644 --- a/vortex-array/src/arrow/executor/list.rs +++ b/vortex-array/src/arrow/executor/list.rs @@ -102,7 +102,7 @@ fn list_to_list( "Cannot convert to non-nullable Arrow array with null elements" ); - let null_buffer = to_arrow_null_buffer(array.validity(), array.len(), ctx)?; + let null_buffer = to_arrow_null_buffer(array.validity()?, array.len(), ctx)?; // TODO(ngates): use new_unchecked when it is added to arrow-rs. Ok(Arc::new(GenericListArray::::new( diff --git a/vortex-array/src/builders/bool.rs b/vortex-array/src/builders/bool.rs index 8dbf4055827..ae4157abcef 100644 --- a/vortex-array/src/builders/bool.rs +++ b/vortex-array/src/builders/bool.rs @@ -195,7 +195,8 @@ mod tests { assert!( canon_into .validity() - .mask_eq(&into_canon.validity(), &mut ctx)? + ? + .mask_eq(&into_canon.validity()?, &mut ctx)? ); assert_eq!(canon_into.to_bit_buffer(), into_canon.to_bit_buffer()); Ok(()) diff --git a/vortex-array/src/builders/fixed_size_list.rs b/vortex-array/src/builders/fixed_size_list.rs index 81142526786..1b14415944d 100644 --- a/vortex-array/src/builders/fixed_size_list.rs +++ b/vortex-array/src/builders/fixed_size_list.rs @@ -272,6 +272,7 @@ mod tests { use std::sync::Arc; use vortex_buffer::buffer; + use vortex_error::VortexExpect; use super::FixedSizeListBuilder; use crate::IntoArray as _; @@ -444,9 +445,9 @@ mod tests { assert_eq!(fsl.len(), 3); let fsl_array = fsl.to_fixed_size_list(); - assert!(fsl_array.validity().is_valid(0).unwrap()); - assert!(!fsl_array.validity().is_valid(1).unwrap()); - assert!(fsl_array.validity().is_valid(2).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); } #[test] @@ -529,7 +530,7 @@ mod tests { // Check that all lists are null. for i in 0..3 { - assert!(!fsl_array.validity().is_valid(i).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(i).unwrap()); } } @@ -552,7 +553,7 @@ mod tests { assert_eq!(fsl_array.list_size(), 2); // Check that all lists are null. - assert!(!fsl_array.validity().is_valid(0).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); } #[test] @@ -621,12 +622,12 @@ mod tests { assert_eq!(fsl_array.elements().len(), 12); // Check validity pattern is repeated. - assert!(fsl_array.validity().is_valid(0).unwrap()); - assert!(!fsl_array.validity().is_valid(1).unwrap()); - assert!(fsl_array.validity().is_valid(2).unwrap()); - assert!(fsl_array.validity().is_valid(3).unwrap()); - assert!(!fsl_array.validity().is_valid(4).unwrap()); - assert!(fsl_array.validity().is_valid(5).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(3).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(4).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(5).unwrap()); } #[test] @@ -661,11 +662,11 @@ mod tests { assert_eq!(fsl_array.elements().len(), 0); // Check validity pattern. - assert!(fsl_array.validity().is_valid(0).unwrap()); - assert!(!fsl_array.validity().is_valid(1).unwrap()); - assert!(fsl_array.validity().is_valid(2).unwrap()); - assert!(!fsl_array.validity().is_valid(3).unwrap()); - assert!(fsl_array.validity().is_valid(4).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(3).unwrap()); + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(4).unwrap()); } #[test] @@ -741,12 +742,12 @@ mod tests { assert_eq!(fsl_array.elements().len(), 12); // Check validity. - assert!(fsl_array.validity().is_valid(0).unwrap()); // append_value - assert!(!fsl_array.validity().is_valid(1).unwrap()); // append_null - assert!(fsl_array.validity().is_valid(2).unwrap()); // append_zeros - assert!(fsl_array.validity().is_valid(3).unwrap()); // append_zeros - assert!(!fsl_array.validity().is_valid(4).unwrap()); // append_nulls - assert!(fsl_array.validity().is_valid(5).unwrap()); // extend_from_array + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); // append_value + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); // append_null + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); // append_zeros + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(3).unwrap()); // append_zeros + assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(4).unwrap()); // append_nulls + assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(5).unwrap()); // extend_from_array } #[test] @@ -789,9 +790,9 @@ mod tests { } // Check validity - first two should be valid, third should be null. - assert!(array.validity().is_valid(0).unwrap()); - assert!(array.validity().is_valid(1).unwrap()); - assert!(!array.validity().is_valid(2).unwrap()); + assert!(array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); + assert!(array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); + assert!(!array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); // Test wrong dtype error. let mut builder = FixedSizeListBuilder::with_capacity(dtype, 2, NonNullable, 10); diff --git a/vortex-array/src/builders/list.rs b/vortex-array/src/builders/list.rs index af8721b7400..ceefcc10632 100644 --- a/vortex-array/src/builders/list.rs +++ b/vortex-array/src/builders/list.rs @@ -311,6 +311,7 @@ mod tests { use Nullability::NonNullable; use Nullability::Nullable; use vortex_buffer::buffer; + use vortex_error::VortexExpect; use crate::IntoArray; use crate::LEGACY_SESSION; @@ -468,7 +469,13 @@ mod tests { assert!( actual .validity() - .mask_eq(&expected.validity(), &mut ctx) + .vortex_expect("list validity should be derivable") + .mask_eq( + &expected + .validity() + .vortex_expect("list validity should be derivable"), + &mut ctx, + ) .unwrap(), ); } @@ -570,9 +577,9 @@ mod tests { assert!(list2.is_null()); // This should be null. // Check validity. - assert!(array.validity().is_valid(0).unwrap()); - assert!(array.validity().is_valid(1).unwrap()); - assert!(!array.validity().is_valid(2).unwrap()); + assert!(array.validity().vortex_expect("list validity should be derivable").is_valid(0).unwrap()); + assert!(array.validity().vortex_expect("list validity should be derivable").is_valid(1).unwrap()); + assert!(!array.validity().vortex_expect("list validity should be derivable").is_valid(2).unwrap()); // Test wrong dtype error. let mut builder = ListBuilder::::with_capacity(dtype, NonNullable, 20, 10); diff --git a/vortex-array/src/builders/listview.rs b/vortex-array/src/builders/listview.rs index 68d1759e159..01a7d7d65ad 100644 --- a/vortex-array/src/builders/listview.rs +++ b/vortex-array/src/builders/listview.rs @@ -429,6 +429,7 @@ mod tests { use std::sync::Arc; use super::ListViewBuilder; + use vortex_error::VortexExpect; use crate::IntoArray; use crate::arrays::ListArray; use crate::assert_arrays_eq; @@ -494,7 +495,7 @@ mod tests { assert_eq!(listview.list_elements_at(1).unwrap().len(), 0); // Check null list. - assert!(!listview.validity().is_valid(2).unwrap()); + assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(2).unwrap()); // Check last list: [4, 5]. assert_arrays_eq!( @@ -595,8 +596,8 @@ mod tests { assert_eq!(listview.list_elements_at(1).unwrap().len(), 0); // Next two are nulls. - assert!(!listview.validity().is_valid(2).unwrap()); - assert!(!listview.validity().is_valid(3).unwrap()); + assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(2).unwrap()); + assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(3).unwrap()); // Last is the regular list: [10, 20]. assert_arrays_eq!( @@ -651,7 +652,7 @@ mod tests { ); // Third list: null (from source). - assert!(!listview.validity().is_valid(2).unwrap()); + assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(2).unwrap()); // Fourth list: [4, 5] (from source). assert_arrays_eq!( diff --git a/vortex-array/src/builders/primitive.rs b/vortex-array/src/builders/primitive.rs index 46221783bda..7c64697434b 100644 --- a/vortex-array/src/builders/primitive.rs +++ b/vortex-array/src/builders/primitive.rs @@ -357,6 +357,7 @@ impl UninitRange<'_, T> { mod tests { use super::*; use crate::assert_arrays_eq; + use vortex_error::VortexExpect; /// REGRESSION TEST: This test verifies that multiple sequential ranges have correct offsets. /// @@ -616,9 +617,9 @@ mod tests { // values[2] might be any value since it's null. // Check validity - first two should be valid, third should be null. - assert!(array.validity().is_valid(0).unwrap()); - assert!(array.validity().is_valid(1).unwrap()); - assert!(!array.validity().is_valid(2).unwrap()); + assert!(array.validity().vortex_expect("primitive validity should be derivable").is_valid(0).unwrap()); + assert!(array.validity().vortex_expect("primitive validity should be derivable").is_valid(1).unwrap()); + assert!(!array.validity().vortex_expect("primitive validity should be derivable").is_valid(2).unwrap()); // Test wrong dtype error. let mut builder = PrimitiveBuilder::::with_capacity(Nullability::NonNullable, 10); diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index e50458bcebc..cb441fa50ce 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -806,7 +806,7 @@ impl Executable for Buffer { fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { let array = PrimitiveArray::execute(array, ctx)?; vortex_ensure!( - matches!(array.validity(), Validity::NonNullable | Validity::AllValid), + matches!(array.validity()?, Validity::NonNullable | Validity::AllValid), "Cannot execute to native buffer: array is not all-valid." ); Ok(array.into_buffer()) diff --git a/vortex-array/src/patches.rs b/vortex-array/src/patches.rs index 2a55df62359..8ae49e21324 100644 --- a/vortex-array/src/patches.rs +++ b/vortex-array/src/patches.rs @@ -746,7 +746,7 @@ impl Patches { include_nulls: bool, ctx: &mut ExecutionCtx, ) -> VortexResult> { - let take_indices_validity = take_indices.validity(); + let take_indices_validity = take_indices.validity()?; let patch_indices = self.indices.clone().execute::(ctx)?; let chunk_offsets = self .chunk_offsets() @@ -833,9 +833,9 @@ impl Patches { match_each_unsigned_integer_ptype!(indices.ptype(), |Indices| { match_each_integer_ptype!(take_indices.ptype(), |TakeIndices| { let take_validity = take_indices - .validity() + .validity()? .execute_mask(take_indices.len(), ctx)?; - let take_nullability = take_indices.validity().nullability(); + let take_nullability = take_indices.validity()?.nullability(); let take_slice = take_indices.as_slice::(); take_map::<_, TakeIndices>( indices.as_slice::(), @@ -892,7 +892,9 @@ impl Patches { .clone() .execute::(ctx) .vortex_expect("patch values must be convertible to PrimitiveArray"); - let patches_validity = patch_values.validity(); + let patches_validity = patch_values + .validity() + .vortex_expect("patch values validity should be derivable"); let patch_values_slice = patch_values.as_slice::

(); match_each_unsigned_integer_ptype!(patch_indices.ptype(), |I| { diff --git a/vortex-array/src/scalar_fn/fns/get_item.rs b/vortex-array/src/scalar_fn/fns/get_item.rs index 320223bb1a3..c86962212b8 100644 --- a/vortex-array/src/scalar_fn/fns/get_item.rs +++ b/vortex-array/src/scalar_fn/fns/get_item.rs @@ -116,7 +116,7 @@ impl ScalarFnVTable for GetItem { match input.dtype().nullability() { Nullability::NonNullable => Ok(field), - Nullability::Nullable => field.mask(input.validity().to_array(input.len())), + Nullability::Nullable => field.mask(input.validity()?.to_array(input.len())), } } diff --git a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs index 46e33176892..55de23b704c 100644 --- a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs +++ b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs @@ -344,7 +344,7 @@ fn list_contains_scalar( Ok(BoolArray::new( list_matches, - list_array.validity().union_nullability(nullability), + list_array.validity()?.union_nullability(nullability), ) .into_array()) } @@ -384,7 +384,7 @@ fn list_false_or_null( list_array: &ListViewArray, nullability: Nullability, ) -> VortexResult { - match list_array.validity() { + match list_array.validity()? { Validity::NonNullable => { // All false. Ok(ConstantArray::new(Scalar::bool(false, nullability), list_array.len()).into_array()) @@ -420,7 +420,7 @@ fn list_is_not_empty( ctx: &mut ExecutionCtx, ) -> VortexResult { // Short-circuit for all invalid. - if matches!(list_array.validity(), Validity::AllInvalid) { + if matches!(list_array.validity()?, Validity::AllInvalid) { return Ok(ConstantArray::new( Scalar::null(DType::Bool(Nullability::Nullable)), list_array.len(), @@ -434,7 +434,7 @@ fn list_is_not_empty( }); // Copy over the validity mask from the input. - Ok(BoolArray::new(buffer, list_array.validity().union_nullability(nullability)).into_array()) + Ok(BoolArray::new(buffer, list_array.validity()?.union_nullability(nullability)).into_array()) } #[cfg(test)] diff --git a/vortex-array/src/scalar_fn/fns/not/mod.rs b/vortex-array/src/scalar_fn/fns/not/mod.rs index bcb7e6a1220..d9c7f96e012 100644 --- a/vortex-array/src/scalar_fn/fns/not/mod.rs +++ b/vortex-array/src/scalar_fn/fns/not/mod.rs @@ -102,7 +102,7 @@ impl ScalarFnVTable for Not { // For boolean array if let Some(bool) = child.as_opt::() { - return Ok(BoolArray::new(!bool.to_bit_buffer(), bool.validity()).into_array()); + return Ok(BoolArray::new(!bool.to_bit_buffer(), bool.validity()?).into_array()); } // Otherwise, execute and try again diff --git a/vortex-array/src/scalar_fn/fns/pack.rs b/vortex-array/src/scalar_fn/fns/pack.rs index e745efff44e..d9fca3822bd 100644 --- a/vortex-array/src/scalar_fn/fns/pack.rs +++ b/vortex-array/src/scalar_fn/fns/pack.rs @@ -229,7 +229,7 @@ mod tests { let actual_array = test_array().apply(&expr).unwrap().to_struct(); assert_eq!(actual_array.names(), ["one", "two", "three"]); - assert!(matches!(actual_array.validity(), Validity::NonNullable)); + assert!(matches!(actual_array.validity(), Ok(Validity::NonNullable))); assert_arrays_eq!( primitive_field(&actual_array.clone().into_array(), &["one"]).unwrap(), @@ -300,7 +300,7 @@ mod tests { let actual_array = test_array().apply(&expr).unwrap().to_struct(); assert_eq!(actual_array.names(), ["one", "two", "three"]); - assert!(matches!(actual_array.validity(), Validity::AllValid)); + assert!(matches!(actual_array.validity(), Ok(Validity::AllValid))); } #[test] diff --git a/vortex-btrblocks/src/schemes/decimal.rs b/vortex-btrblocks/src/schemes/decimal.rs index bcf59c13db7..fb35b870702 100644 --- a/vortex-btrblocks/src/schemes/decimal.rs +++ b/vortex-btrblocks/src/schemes/decimal.rs @@ -60,7 +60,7 @@ impl Scheme for DecimalScheme { // for compression. 2 for i128 and 4 for i256. let decimal = data.array().clone().to_decimal(); let decimal = narrowed_decimal(decimal); - let validity = decimal.validity(); + let validity = decimal.validity()?; let prim = match decimal.values_type() { DecimalType::I8 => PrimitiveArray::new(decimal.buffer::(), validity), DecimalType::I16 => PrimitiveArray::new(decimal.buffer::(), validity), diff --git a/vortex-btrblocks/src/schemes/integer.rs b/vortex-btrblocks/src/schemes/integer.rs index 9309637c158..f0211d5b7da 100644 --- a/vortex-btrblocks/src/schemes/integer.rs +++ b/vortex-btrblocks/src/schemes/integer.rs @@ -794,7 +794,7 @@ mod tests { false, false, false, false, false, false, false, false, false, false, true, ]), ); - let validity = array.validity(); + let validity = array.validity()?; let btr = BtrBlocksCompressor::default(); let compressed = btr.compress(&array.into_array())?; diff --git a/vortex-btrblocks/src/schemes/string.rs b/vortex-btrblocks/src/schemes/string.rs index f222ce1623b..b4e15b6c76b 100644 --- a/vortex-btrblocks/src/schemes/string.rs +++ b/vortex-btrblocks/src/schemes/string.rs @@ -103,7 +103,7 @@ impl Scheme for FSSTScheme { compressed_codes_offsets, fsst.codes().bytes().clone(), fsst.codes().dtype().clone(), - fsst.codes().validity(), + fsst.codes().validity()?, )?; let fsst = FSST::try_new( diff --git a/vortex-compressor/src/builtins/constant.rs b/vortex-compressor/src/builtins/constant.rs index 53300d49703..767ffaa9167 100644 --- a/vortex-compressor/src/builtins/constant.rs +++ b/vortex-compressor/src/builtins/constant.rs @@ -230,7 +230,7 @@ impl Scheme for StringConstantScheme { let scalar = stats.source().scalar_at(idx)?; let const_arr = ConstantArray::new(scalar, stats.source().len()).into_array(); if !stats.source().all_valid()? { - Ok(MaskedArray::try_new(const_arr, stats.source().validity())?.into_array()) + Ok(MaskedArray::try_new(const_arr, stats.source().validity()?)?.into_array()) } else { Ok(const_arr) } @@ -253,7 +253,7 @@ fn compress_constant_primitive(source: &PrimitiveArray) -> VortexResult DictArray { - let validity = stats.source().validity(); + let validity = stats + .source() + .validity() + .vortex_expect("dictionary source validity should be derivable"); match stats.erased() { FloatErasedStats::F16(typed) => typed_encode!(stats, typed, validity, f16), FloatErasedStats::F32(typed) => typed_encode!(stats, typed, validity, f32), diff --git a/vortex-compressor/src/builtins/dict/integer.rs b/vortex-compressor/src/builtins/dict/integer.rs index 04842237930..1e651a81861 100644 --- a/vortex-compressor/src/builtins/dict/integer.rs +++ b/vortex-compressor/src/builtins/dict/integer.rs @@ -63,7 +63,10 @@ macro_rules! typed_encode { reason = "complexity from match on all integer types" )] pub fn dictionary_encode(stats: &IntegerStats) -> DictArray { - let src_validity = stats.source().validity(); + let src_validity = stats + .source() + .validity() + .vortex_expect("dictionary source validity should be derivable"); match stats.erased() { IntegerErasedStats::U8(typed) => typed_encode!(stats, typed, src_validity, u8), diff --git a/vortex-compressor/src/compressor.rs b/vortex-compressor/src/compressor.rs index dafe88fdd07..a7594feac4a 100644 --- a/vortex-compressor/src/compressor.rs +++ b/vortex-compressor/src/compressor.rs @@ -189,7 +189,7 @@ impl CascadingCompressor { struct_array.names().clone(), fields, struct_array.len(), - struct_array.validity(), + struct_array.validity()?, )? .into_array()) } @@ -207,7 +207,7 @@ impl CascadingCompressor { Ok(FixedSizeListArray::try_new( compressed_elems, fsl_array.list_size(), - fsl_array.validity(), + fsl_array.validity()?, fsl_array.len(), )? .into_array()) @@ -450,7 +450,7 @@ impl CascadingCompressor { compressed_elems, compressed_offsets, compressed_sizes, - list_view.validity(), + list_view.validity()?, )? .into_array()) } diff --git a/vortex-cuda/src/arrow/canonical.rs b/vortex-cuda/src/arrow/canonical.rs index 605d2a9e356..4e38efe939e 100644 --- a/vortex-cuda/src/arrow/canonical.rs +++ b/vortex-cuda/src/arrow/canonical.rs @@ -142,7 +142,7 @@ fn export_canonical( } Canonical::VarBinView(varbinview) => { let len = varbinview.len(); - check_validity_empty(&varbinview.validity())?; + check_validity_empty(&varbinview.validity()?)?; let BinaryParts { offsets, bytes } = copy_varbinview_to_varbin(varbinview, ctx).await?; diff --git a/vortex-cuda/src/kernel/arrays/dict.rs b/vortex-cuda/src/kernel/arrays/dict.rs index a8437e0b85b..c13d8196294 100644 --- a/vortex-cuda/src/kernel/arrays/dict.rs +++ b/vortex-cuda/src/kernel/arrays/dict.rs @@ -316,7 +316,7 @@ mod tests { Ok(PrimitiveArray::from_byte_buffer( prim.buffer_handle().try_to_host_sync()?, prim.ptype(), - prim.validity(), + prim.validity()?, )) } @@ -647,7 +647,7 @@ mod tests { BufferHandle::new_host(decimal.buffer_handle().try_to_host_sync()?), decimal.values_type(), decimal.decimal_dtype(), - decimal.validity(), + decimal.validity()?, )) } diff --git a/vortex-cuda/src/kernel/patches/mod.rs b/vortex-cuda/src/kernel/patches/mod.rs index 5d833c1c417..3d611ee4759 100644 --- a/vortex-cuda/src/kernel/patches/mod.rs +++ b/vortex-cuda/src/kernel/patches/mod.rs @@ -42,7 +42,7 @@ pub(crate) async fn execute_patches< let values = values.execute_cuda(ctx).await?.into_primitive(); let supported = matches!( - values.validity(), + values.validity()?, Validity::NonNullable | Validity::AllValid ); vortex_ensure!( diff --git a/vortex-duckdb/src/exporter/bool.rs b/vortex-duckdb/src/exporter/bool.rs index 4e6277df3b8..75dfa195bc9 100644 --- a/vortex-duckdb/src/exporter/bool.rs +++ b/vortex-duckdb/src/exporter/bool.rs @@ -23,7 +23,7 @@ pub(crate) fn new_exporter( ) -> VortexResult> { let len = array.len(); let bits = array.to_bit_buffer(); - let validity = array.validity().to_array(len).execute::(ctx)?; + let validity = array.validity()?.to_array(len).execute::(ctx)?; if validity.all_false() { return Ok(all_invalid::new_exporter(len, &LogicalType::bool())); diff --git a/vortex-duckdb/src/exporter/mod.rs b/vortex-duckdb/src/exporter/mod.rs index b3f5d12ecf0..6b4069f701c 100644 --- a/vortex-duckdb/src/exporter/mod.rs +++ b/vortex-duckdb/src/exporter/mod.rs @@ -56,7 +56,7 @@ impl ArrayExporter { cache: &ConversionCache, mut ctx: ExecutionCtx, ) -> VortexResult { - let validity = array.validity().execute_mask(array.len(), &mut ctx)?; + let validity = array.validity()?.execute_mask(array.len(), &mut ctx)?; assert!(validity.all_true()); let fields = array diff --git a/vortex-duckdb/src/exporter/primitive.rs b/vortex-duckdb/src/exporter/primitive.rs index c5167880b59..7506cbdb93c 100644 --- a/vortex-duckdb/src/exporter/primitive.rs +++ b/vortex-duckdb/src/exporter/primitive.rs @@ -29,7 +29,7 @@ pub fn new_exporter( ctx: &mut ExecutionCtx, ) -> VortexResult> { let validity = array - .validity() + .validity()? .to_array(array.len()) .execute::(ctx)?; diff --git a/vortex-layout/src/layouts/struct_/reader.rs b/vortex-layout/src/layouts/struct_/reader.rs index 7cffc587900..0d803cdd4c7 100644 --- a/vortex-layout/src/layouts/struct_/reader.rs +++ b/vortex-layout/src/layouts/struct_/reader.rs @@ -370,7 +370,7 @@ impl LayoutReader for StructReader { struct_array.names().clone(), masked_fields, struct_array.len(), - struct_array.validity(), + struct_array.validity()?, )? .into_array()) } else { diff --git a/vortex/benches/common_encoding_tree_throughput.rs b/vortex/benches/common_encoding_tree_throughput.rs index 21fc4e74b37..682a73296f9 100644 --- a/vortex/benches/common_encoding_tree_throughput.rs +++ b/vortex/benches/common_encoding_tree_throughput.rs @@ -18,6 +18,7 @@ use vortex::array::IntoArray; use vortex::array::ToCanonical; use vortex::array::arrays::DictArray; use vortex::array::arrays::PrimitiveArray; +use vortex::error::VortexExpect; use vortex::array::arrays::TemporalArray; use vortex::array::arrays::VarBinArray; use vortex::array::arrays::VarBinViewArray; @@ -262,7 +263,9 @@ mod setup { offsets_bp.into_array(), codes.bytes().clone(), codes.dtype().clone(), - codes.validity(), + codes + .validity() + .vortex_expect("FSST code validity should be derivable"), ) .unwrap(); diff --git a/vortex/src/lib.rs b/vortex/src/lib.rs index c84055d92aa..9053dbc7505 100644 --- a/vortex/src/lib.rs +++ b/vortex/src/lib.rs @@ -336,8 +336,8 @@ mod test { let recovered_primitive = recovered_array.execute::(&mut ctx)?; assert!( recovered_primitive - .validity() - .mask_eq(&array.validity(), &mut ctx)? + .validity()? + .mask_eq(&array.validity()?, &mut ctx)? ); assert_eq!( recovered_primitive.to_buffer::(), From 0092911c97c8dc78b21816e54f557d376cee3955 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Fri, 3 Apr 2026 16:51:25 -0400 Subject: [PATCH 2/2] Fix Array::validity Signed-off-by: Nicholas Gates --- encodings/bytebool/public-api.lock | 12 +- encodings/bytebool/src/compute.rs | 4 +- encodings/datetime-parts/src/canonical.rs | 7 +- encodings/parquet-variant/src/validity.rs | 3 +- encodings/pco/src/test.rs | 1 + encodings/zstd/src/test.rs | 4 +- vortex-array/public-api.lock | 40 +--- vortex-array/src/array/vtable/validity.rs | 2 +- .../src/arrays/filter/execute/bool.rs | 4 +- .../src/arrays/masked/compute/mask.rs | 3 +- .../src/arrays/primitive/array/accessor.rs | 5 +- .../src/arrays/primitive/compute/take/mod.rs | 4 +- vortex-array/src/builders/bool.rs | 3 +- vortex-array/src/builders/fixed_size_list.rs | 200 +++++++++++++++--- vortex-array/src/builders/list.rs | 24 ++- vortex-array/src/builders/listview.rs | 35 ++- vortex-array/src/builders/primitive.rs | 27 ++- vortex-array/src/canonical.rs | 5 +- .../src/scalar_fn/fns/list_contains/mod.rs | 6 +- .../common_encoding_tree_throughput.rs | 2 +- 20 files changed, 297 insertions(+), 94 deletions(-) diff --git a/encodings/bytebool/public-api.lock b/encodings/bytebool/public-api.lock index 74eed2ce798..f626040d55d 100644 --- a/encodings/bytebool/public-api.lock +++ b/encodings/bytebool/public-api.lock @@ -26,7 +26,7 @@ pub type vortex_bytebool::ByteBool::ArrayData = vortex_bytebool::ByteBoolData pub type vortex_bytebool::ByteBool::OperationsVTable = vortex_bytebool::ByteBool -pub type vortex_bytebool::ByteBool::ValidityVTable = vortex_array::array::vtable::validity::ValidityVTableFromValidityHelper +pub type vortex_bytebool::ByteBool::ValidityVTable = vortex_bytebool::ByteBool pub fn vortex_bytebool::ByteBool::array_eq(array: &vortex_bytebool::ByteBoolData, other: &vortex_bytebool::ByteBoolData, precision: vortex_array::hash::Precision) -> bool @@ -62,6 +62,10 @@ impl vortex_array::array::vtable::operations::OperationsVTable, index: usize, _ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult +impl vortex_array::array::vtable::validity::ValidityVTable for vortex_bytebool::ByteBool + +pub fn vortex_bytebool::ByteBool::validity(array: vortex_array::array::view::ArrayView<'_, vortex_bytebool::ByteBool>) -> vortex_error::VortexResult + impl vortex_array::arrays::dict::take::TakeExecute for vortex_bytebool::ByteBool pub fn vortex_bytebool::ByteBool::take(array: vortex_array::array::view::ArrayView<'_, Self>, indices: &vortex_array::array::erased::ArrayRef, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult> @@ -96,6 +100,8 @@ pub fn vortex_bytebool::ByteBoolData::new(buffer: vortex_array::buffer::BufferHa pub fn vortex_bytebool::ByteBoolData::validate(buffer: &vortex_array::buffer::BufferHandle, validity: &vortex_array::validity::Validity, dtype: &vortex_array::dtype::DType, len: usize) -> vortex_error::VortexResult<()> +pub fn vortex_bytebool::ByteBoolData::validity(&self) -> vortex_array::validity::Validity + pub fn vortex_bytebool::ByteBoolData::validity_mask(&self) -> vortex_mask::Mask impl core::clone::Clone for vortex_bytebool::ByteBoolData @@ -114,8 +120,4 @@ impl core::fmt::Debug for vortex_bytebool::ByteBoolData pub fn vortex_bytebool::ByteBoolData::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl vortex_array::array::vtable::validity::ValidityHelper for vortex_bytebool::ByteBoolData - -pub fn vortex_bytebool::ByteBoolData::validity(&self) -> &vortex_array::validity::Validity - pub type vortex_bytebool::ByteBoolArray = vortex_array::array::typed::Array diff --git a/encodings/bytebool/src/compute.rs b/encodings/bytebool/src/compute.rs index ebb0bdbc4bf..0fd75cb8123 100644 --- a/encodings/bytebool/src/compute.rs +++ b/encodings/bytebool/src/compute.rs @@ -25,7 +25,9 @@ impl CastReduce for ByteBool { // If just changing nullability, we can optimize if array.dtype().eq_ignore_nullability(dtype) { - let new_validity = array.validity()?.cast_nullability(dtype.nullability(), array.len())?; + let new_validity = array + .validity()? + .cast_nullability(dtype.nullability(), array.len())?; return Ok(Some( ByteBool::new(array.buffer().clone(), new_validity).into_array(), diff --git a/encodings/datetime-parts/src/canonical.rs b/encodings/datetime-parts/src/canonical.rs index daecaecfd3f..c5b969278c2 100644 --- a/encodings/datetime-parts/src/canonical.rs +++ b/encodings/datetime-parts/src/canonical.rs @@ -160,7 +160,12 @@ mod test { .execute::(&mut ctx)?; assert_arrays_eq!(primitive_values, milliseconds); - assert!(primitive_values.validity().mask_eq(&validity, &mut ctx)?); + assert!( + primitive_values + .validity() + .unwrap() + .mask_eq(&validity, &mut ctx)? + ); Ok(()) } } diff --git a/encodings/parquet-variant/src/validity.rs b/encodings/parquet-variant/src/validity.rs index 2dde87a18f1..50994d8b8a7 100644 --- a/encodings/parquet-variant/src/validity.rs +++ b/encodings/parquet-variant/src/validity.rs @@ -1,11 +1,10 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -use vortex_error::VortexResult; - use vortex_array::ArrayView; use vortex_array::validity::Validity; use vortex_array::vtable::ValidityVTable; +use vortex_error::VortexResult; use crate::vtable::ParquetVariant; diff --git a/encodings/pco/src/test.rs b/encodings/pco/src/test.rs index 0bcbd8837ee..28c4cdbf830 100644 --- a/encodings/pco/src/test.rs +++ b/encodings/pco/src/test.rs @@ -131,6 +131,7 @@ fn test_validity_and_multiple_chunks_and_pages() { assert!( primitive .validity() + .unwrap() .mask_eq( &Validity::Array(BoolArray::from_iter(vec![true, false, true]).into_array()), &mut ctx, diff --git a/encodings/zstd/src/test.rs b/encodings/zstd/src/test.rs index 7548f32cb80..01800577d7b 100644 --- a/encodings/zstd/src/test.rs +++ b/encodings/zstd/src/test.rs @@ -87,7 +87,8 @@ fn test_zstd_with_validity_and_multi_frame() { assert!( decompressed .validity() - .mask_eq(&array.validity(), &mut ctx) + .unwrap() + .mask_eq(&array.validity().unwrap(), &mut ctx) .unwrap() ); @@ -101,6 +102,7 @@ fn test_zstd_with_validity_and_multi_frame() { assert!( primitive .validity() + .unwrap() .mask_eq( &Validity::Array(BoolArray::from_iter(vec![false, true, false]).into_array()), &mut ctx diff --git a/vortex-array/public-api.lock b/vortex-array/public-api.lock index 7e6487c34d1..265b63977f6 100644 --- a/vortex-array/public-api.lock +++ b/vortex-array/public-api.lock @@ -18774,12 +18774,6 @@ impl vortex_array::ValidityVTable for vortex_array:: pub fn vortex_array::ValidityVTableFromChildSliceHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult -pub struct vortex_array::vtable::ValidityVTableFromValidityHelper - -impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValidityHelper where ::ArrayData: vortex_array::ValidityHelper - -pub fn vortex_array::ValidityVTableFromValidityHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult - pub struct vortex_array::vtable::ValidityVTableFromValiditySliceHelper impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValiditySliceHelper where ::ArrayData: vortex_array::ValiditySliceHelper @@ -21128,10 +21122,6 @@ pub fn vortex_array::vtable::ValidityChildSliceHelper::sliced_child_array(&self) pub fn vortex_array::vtable::ValidityChildSliceHelper::unsliced_child_and_slice(&self) -> (&vortex_array::ArrayRef, usize, usize) -pub trait vortex_array::vtable::ValidityHelper - -pub fn vortex_array::vtable::ValidityHelper::validity(&self) -> &vortex_array::validity::Validity - pub trait vortex_array::vtable::ValiditySliceHelper pub fn vortex_array::vtable::ValiditySliceHelper::sliced_validity(&self) -> vortex_error::VortexResult @@ -21222,10 +21212,6 @@ impl vortex_array::ValidityVTable for vortex_array:: pub fn vortex_array::ValidityVTableFromChildSliceHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult -impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValidityHelper where ::ArrayData: vortex_array::ValidityHelper - -pub fn vortex_array::ValidityVTableFromValidityHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult - impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValiditySliceHelper where ::ArrayData: vortex_array::ValiditySliceHelper pub fn vortex_array::ValidityVTableFromValiditySliceHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult @@ -21750,10 +21736,6 @@ pub fn vortex_array::Array::new(child: vorte pub fn vortex_array::Array::try_new(child: vortex_array::ArrayRef, range: core::ops::range::Range) -> vortex_error::VortexResult -impl vortex_array::Array where ::ArrayData: vortex_array::ValidityHelper - -pub fn vortex_array::Array::validity(&self) -> &vortex_array::validity::Validity - impl vortex_array::Array pub fn vortex_array::Array::all_invalid(&self) -> vortex_error::VortexResult @@ -21786,6 +21768,8 @@ pub fn vortex_array::Array::to_canonical(&self) -> vortex_error::VortexResult pub fn vortex_array::Array::valid_count(&self) -> vortex_error::VortexResult +pub fn vortex_array::Array::validity(&self) -> vortex_error::VortexResult + pub fn vortex_array::Array::validity_mask(&self) -> vortex_error::VortexResult impl vortex_array::Array @@ -22400,10 +22384,6 @@ pub fn vortex_array::ArrayRef::from(value: vortex_array::Array) -> vortex_arr pub struct vortex_array::ArrayView<'a, V: vortex_array::VTable> -impl<'a, V: vortex_array::VTable> vortex_array::ArrayView<'a, V> where ::ArrayData: vortex_array::ValidityHelper - -pub fn vortex_array::ArrayView<'a, V>::validity(&self) -> &'a vortex_array::validity::Validity - impl<'a, V: vortex_array::VTable> vortex_array::ArrayView<'a, V> pub fn vortex_array::ArrayView<'a, V>::array(&self) -> &'a vortex_array::ArrayRef @@ -22422,6 +22402,8 @@ pub fn vortex_array::ArrayView<'a, V>::len(&self) -> usize pub fn vortex_array::ArrayView<'a, V>::statistics(&self) -> vortex_array::stats::StatsSetRef<'_> +pub fn vortex_array::ArrayView<'a, V>::validity(&self) -> vortex_error::VortexResult + impl core::clone::Clone for vortex_array::ArrayView<'_, V> pub fn vortex_array::ArrayView<'_, V>::clone(&self) -> Self @@ -22606,12 +22588,6 @@ impl vortex_array::ValidityVTable for vortex_array:: pub fn vortex_array::ValidityVTableFromChildSliceHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult -pub struct vortex_array::ValidityVTableFromValidityHelper - -impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValidityHelper where ::ArrayData: vortex_array::ValidityHelper - -pub fn vortex_array::ValidityVTableFromValidityHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult - pub struct vortex_array::ValidityVTableFromValiditySliceHelper impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValiditySliceHelper where ::ArrayData: vortex_array::ValiditySliceHelper @@ -25274,10 +25250,6 @@ pub fn vortex_array::ValidityChildSliceHelper::sliced_child_array(&self) -> vort pub fn vortex_array::ValidityChildSliceHelper::unsliced_child_and_slice(&self) -> (&vortex_array::ArrayRef, usize, usize) -pub trait vortex_array::ValidityHelper - -pub fn vortex_array::ValidityHelper::validity(&self) -> &vortex_array::validity::Validity - pub trait vortex_array::ValiditySliceHelper pub fn vortex_array::ValiditySliceHelper::sliced_validity(&self) -> vortex_error::VortexResult @@ -25368,10 +25340,6 @@ impl vortex_array::ValidityVTable for vortex_array:: pub fn vortex_array::ValidityVTableFromChildSliceHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult -impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValidityHelper where ::ArrayData: vortex_array::ValidityHelper - -pub fn vortex_array::ValidityVTableFromValidityHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult - impl vortex_array::ValidityVTable for vortex_array::ValidityVTableFromValiditySliceHelper where ::ArrayData: vortex_array::ValiditySliceHelper pub fn vortex_array::ValidityVTableFromValiditySliceHelper::validity(array: vortex_array::ArrayView<'_, V>) -> vortex_error::VortexResult diff --git a/vortex-array/src/array/vtable/validity.rs b/vortex-array/src/array/vtable/validity.rs index 3ca11cdca9d..02eac298f46 100644 --- a/vortex-array/src/array/vtable/validity.rs +++ b/vortex-array/src/array/vtable/validity.rs @@ -9,7 +9,7 @@ use crate::array::VTable; use crate::validity::Validity; pub trait ValidityVTable { -/// Returns the [`Validity`] of the array. + /// Returns the [`Validity`] of the array. /// /// ## Pre-conditions /// diff --git a/vortex-array/src/arrays/filter/execute/bool.rs b/vortex-array/src/arrays/filter/execute/bool.rs index e054ca7ed30..e6151a6d770 100644 --- a/vortex-array/src/arrays/filter/execute/bool.rs +++ b/vortex-array/src/arrays/filter/execute/bool.rs @@ -11,7 +11,9 @@ use crate::arrays::filter::execute::bitbuffer; use crate::arrays::filter::execute::filter_validity; pub fn filter_bool(array: &BoolArray, mask: &Arc) -> BoolArray { - let validity = array.validity().vortex_expect("bool validity should be derivable"); + let validity = array + .validity() + .vortex_expect("bool validity should be derivable"); let filtered_validity = filter_validity(validity, mask); let bit_buffer = array.to_bit_buffer(); diff --git a/vortex-array/src/arrays/masked/compute/mask.rs b/vortex-array/src/arrays/masked/compute/mask.rs index c80a1a05082..f822e909432 100644 --- a/vortex-array/src/arrays/masked/compute/mask.rs +++ b/vortex-array/src/arrays/masked/compute/mask.rs @@ -16,8 +16,7 @@ impl MaskReduce for Masked { fn mask(array: ArrayView<'_, Masked>, mask: &ArrayRef) -> VortexResult> { // AND the existing validity mask with the new mask and push into child. let combined_mask = array - .validity() - ? + .validity()? .and(Validity::Array(mask.clone()))? .to_array(array.len()); let masked_child = MaskExpr.try_new_array( diff --git a/vortex-array/src/arrays/primitive/array/accessor.rs b/vortex-array/src/arrays/primitive/array/accessor.rs index c8759d472b2..5a3665c71ed 100644 --- a/vortex-array/src/arrays/primitive/array/accessor.rs +++ b/vortex-array/src/arrays/primitive/array/accessor.rs @@ -16,7 +16,10 @@ impl ArrayAccessor for PrimitiveArray { where F: for<'a> FnOnce(&mut dyn Iterator>) -> R, { - match self.validity().vortex_expect("primitive validity should be derivable") { + match self + .validity() + .vortex_expect("primitive validity should be derivable") + { Validity::NonNullable | Validity::AllValid => { let mut iter = self.as_slice::().iter().map(Some); f(&mut iter) diff --git a/vortex-array/src/arrays/primitive/compute/take/mod.rs b/vortex-array/src/arrays/primitive/compute/take/mod.rs index 8afd7dbcf43..230595039fa 100644 --- a/vortex-array/src/arrays/primitive/compute/take/mod.rs +++ b/vortex-array/src/arrays/primitive/compute/take/mod.rs @@ -99,7 +99,9 @@ impl TakeExecute for Primitive { .execute::(ctx)? }; - let validity = array.validity()?.take(&unsigned_indices.clone().into_array())?; + let validity = array + .validity()? + .take(&unsigned_indices.clone().into_array())?; // Delegate to the best kernel based on the target CPU { let unsigned_indices = unsigned_indices.as_view(); diff --git a/vortex-array/src/builders/bool.rs b/vortex-array/src/builders/bool.rs index ae4157abcef..fa4073de98c 100644 --- a/vortex-array/src/builders/bool.rs +++ b/vortex-array/src/builders/bool.rs @@ -194,8 +194,7 @@ mod tests { assert!( canon_into - .validity() - ? + .validity()? .mask_eq(&into_canon.validity()?, &mut ctx)? ); assert_eq!(canon_into.to_bit_buffer(), into_canon.to_bit_buffer()); diff --git a/vortex-array/src/builders/fixed_size_list.rs b/vortex-array/src/builders/fixed_size_list.rs index 1b14415944d..ad8a5e91214 100644 --- a/vortex-array/src/builders/fixed_size_list.rs +++ b/vortex-array/src/builders/fixed_size_list.rs @@ -445,9 +445,27 @@ mod tests { assert_eq!(fsl.len(), 3); let fsl_array = fsl.to_fixed_size_list(); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(0) + .unwrap() + ); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(1) + .unwrap() + ); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(2) + .unwrap() + ); } #[test] @@ -530,7 +548,13 @@ mod tests { // Check that all lists are null. for i in 0..3 { - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(i).unwrap()); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(i) + .unwrap() + ); } } @@ -553,7 +577,13 @@ mod tests { assert_eq!(fsl_array.list_size(), 2); // Check that all lists are null. - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(0) + .unwrap() + ); } #[test] @@ -622,12 +652,48 @@ mod tests { assert_eq!(fsl_array.elements().len(), 12); // Check validity pattern is repeated. - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(3).unwrap()); - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(4).unwrap()); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(5).unwrap()); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(0) + .unwrap() + ); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(1) + .unwrap() + ); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(2) + .unwrap() + ); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(3) + .unwrap() + ); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(4) + .unwrap() + ); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(5) + .unwrap() + ); } #[test] @@ -662,11 +728,41 @@ mod tests { assert_eq!(fsl_array.elements().len(), 0); // Check validity pattern. - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(3).unwrap()); - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(4).unwrap()); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(0) + .unwrap() + ); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(1) + .unwrap() + ); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(2) + .unwrap() + ); + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(3) + .unwrap() + ); + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(4) + .unwrap() + ); } #[test] @@ -742,12 +838,48 @@ mod tests { assert_eq!(fsl_array.elements().len(), 12); // Check validity. - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); // append_value - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); // append_null - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); // append_zeros - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(3).unwrap()); // append_zeros - assert!(!fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(4).unwrap()); // append_nulls - assert!(fsl_array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(5).unwrap()); // extend_from_array + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(0) + .unwrap() + ); // append_value + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(1) + .unwrap() + ); // append_null + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(2) + .unwrap() + ); // append_zeros + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(3) + .unwrap() + ); // append_zeros + assert!( + !fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(4) + .unwrap() + ); // append_nulls + assert!( + fsl_array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(5) + .unwrap() + ); // extend_from_array } #[test] @@ -790,9 +922,27 @@ mod tests { } // Check validity - first two should be valid, third should be null. - assert!(array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(0).unwrap()); - assert!(array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(1).unwrap()); - assert!(!array.validity().vortex_expect("fixed-size-list validity should be derivable").is_valid(2).unwrap()); + assert!( + array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(0) + .unwrap() + ); + assert!( + array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(1) + .unwrap() + ); + assert!( + !array + .validity() + .vortex_expect("fixed-size-list validity should be derivable") + .is_valid(2) + .unwrap() + ); // Test wrong dtype error. let mut builder = FixedSizeListBuilder::with_capacity(dtype, 2, NonNullable, 10); diff --git a/vortex-array/src/builders/list.rs b/vortex-array/src/builders/list.rs index ceefcc10632..b29bbedce58 100644 --- a/vortex-array/src/builders/list.rs +++ b/vortex-array/src/builders/list.rs @@ -577,9 +577,27 @@ mod tests { assert!(list2.is_null()); // This should be null. // Check validity. - assert!(array.validity().vortex_expect("list validity should be derivable").is_valid(0).unwrap()); - assert!(array.validity().vortex_expect("list validity should be derivable").is_valid(1).unwrap()); - assert!(!array.validity().vortex_expect("list validity should be derivable").is_valid(2).unwrap()); + assert!( + array + .validity() + .vortex_expect("list validity should be derivable") + .is_valid(0) + .unwrap() + ); + assert!( + array + .validity() + .vortex_expect("list validity should be derivable") + .is_valid(1) + .unwrap() + ); + assert!( + !array + .validity() + .vortex_expect("list validity should be derivable") + .is_valid(2) + .unwrap() + ); // Test wrong dtype error. let mut builder = ListBuilder::::with_capacity(dtype, NonNullable, 20, 10); diff --git a/vortex-array/src/builders/listview.rs b/vortex-array/src/builders/listview.rs index 01a7d7d65ad..d88cf5558c0 100644 --- a/vortex-array/src/builders/listview.rs +++ b/vortex-array/src/builders/listview.rs @@ -428,8 +428,9 @@ fn adjust_and_extend_offsets<'a, O: IntegerPType, A: IntegerPType>( mod tests { use std::sync::Arc; - use super::ListViewBuilder; use vortex_error::VortexExpect; + + use super::ListViewBuilder; use crate::IntoArray; use crate::arrays::ListArray; use crate::assert_arrays_eq; @@ -495,7 +496,13 @@ mod tests { assert_eq!(listview.list_elements_at(1).unwrap().len(), 0); // Check null list. - assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(2).unwrap()); + assert!( + !listview + .validity() + .vortex_expect("listview validity should be derivable") + .is_valid(2) + .unwrap() + ); // Check last list: [4, 5]. assert_arrays_eq!( @@ -596,8 +603,20 @@ mod tests { assert_eq!(listview.list_elements_at(1).unwrap().len(), 0); // Next two are nulls. - assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(2).unwrap()); - assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(3).unwrap()); + assert!( + !listview + .validity() + .vortex_expect("listview validity should be derivable") + .is_valid(2) + .unwrap() + ); + assert!( + !listview + .validity() + .vortex_expect("listview validity should be derivable") + .is_valid(3) + .unwrap() + ); // Last is the regular list: [10, 20]. assert_arrays_eq!( @@ -652,7 +671,13 @@ mod tests { ); // Third list: null (from source). - assert!(!listview.validity().vortex_expect("listview validity should be derivable").is_valid(2).unwrap()); + assert!( + !listview + .validity() + .vortex_expect("listview validity should be derivable") + .is_valid(2) + .unwrap() + ); // Fourth list: [4, 5] (from source). assert_arrays_eq!( diff --git a/vortex-array/src/builders/primitive.rs b/vortex-array/src/builders/primitive.rs index 7c64697434b..8321c03500c 100644 --- a/vortex-array/src/builders/primitive.rs +++ b/vortex-array/src/builders/primitive.rs @@ -355,9 +355,10 @@ impl UninitRange<'_, T> { #[cfg(test)] mod tests { + use vortex_error::VortexExpect; + use super::*; use crate::assert_arrays_eq; - use vortex_error::VortexExpect; /// REGRESSION TEST: This test verifies that multiple sequential ranges have correct offsets. /// @@ -617,9 +618,27 @@ mod tests { // values[2] might be any value since it's null. // Check validity - first two should be valid, third should be null. - assert!(array.validity().vortex_expect("primitive validity should be derivable").is_valid(0).unwrap()); - assert!(array.validity().vortex_expect("primitive validity should be derivable").is_valid(1).unwrap()); - assert!(!array.validity().vortex_expect("primitive validity should be derivable").is_valid(2).unwrap()); + assert!( + array + .validity() + .vortex_expect("primitive validity should be derivable") + .is_valid(0) + .unwrap() + ); + assert!( + array + .validity() + .vortex_expect("primitive validity should be derivable") + .is_valid(1) + .unwrap() + ); + assert!( + !array + .validity() + .vortex_expect("primitive validity should be derivable") + .is_valid(2) + .unwrap() + ); // Test wrong dtype error. let mut builder = PrimitiveBuilder::::with_capacity(Nullability::NonNullable, 10); diff --git a/vortex-array/src/canonical.rs b/vortex-array/src/canonical.rs index cb441fa50ce..d48b5d532a2 100644 --- a/vortex-array/src/canonical.rs +++ b/vortex-array/src/canonical.rs @@ -806,7 +806,10 @@ impl Executable for Buffer { fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { let array = PrimitiveArray::execute(array, ctx)?; vortex_ensure!( - matches!(array.validity()?, Validity::NonNullable | Validity::AllValid), + matches!( + array.validity()?, + Validity::NonNullable | Validity::AllValid + ), "Cannot execute to native buffer: array is not all-valid." ); Ok(array.into_buffer()) diff --git a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs index 55de23b704c..6bc07e93e71 100644 --- a/vortex-array/src/scalar_fn/fns/list_contains/mod.rs +++ b/vortex-array/src/scalar_fn/fns/list_contains/mod.rs @@ -434,7 +434,11 @@ fn list_is_not_empty( }); // Copy over the validity mask from the input. - Ok(BoolArray::new(buffer, list_array.validity()?.union_nullability(nullability)).into_array()) + Ok(BoolArray::new( + buffer, + list_array.validity()?.union_nullability(nullability), + ) + .into_array()) } #[cfg(test)] diff --git a/vortex/benches/common_encoding_tree_throughput.rs b/vortex/benches/common_encoding_tree_throughput.rs index 682a73296f9..6f48a19de3a 100644 --- a/vortex/benches/common_encoding_tree_throughput.rs +++ b/vortex/benches/common_encoding_tree_throughput.rs @@ -18,7 +18,6 @@ use vortex::array::IntoArray; use vortex::array::ToCanonical; use vortex::array::arrays::DictArray; use vortex::array::arrays::PrimitiveArray; -use vortex::error::VortexExpect; use vortex::array::arrays::TemporalArray; use vortex::array::arrays::VarBinArray; use vortex::array::arrays::VarBinViewArray; @@ -35,6 +34,7 @@ use vortex::encodings::fsst::FSST; use vortex::encodings::fsst::fsst_compress; use vortex::encodings::fsst::fsst_train_compressor; use vortex::encodings::runend::RunEnd; +use vortex::error::VortexExpect; use vortex::extension::datetime::TimeUnit; #[global_allocator]