diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index ebcc0f7bb6..620574c951 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -207,7 +207,8 @@ impl IntoCanonical for ALPRDArray { right_parts.into_buffer_mut::(), self.left_parts_patches(), )?, - self.logical_validity().into_validity(), + self.logical_validity() + .into_validity(self.dtype().nullability()), ) } else { PrimitiveArray::new( @@ -218,7 +219,8 @@ impl IntoCanonical for ALPRDArray { right_parts.into_buffer_mut::(), self.left_parts_patches(), )?, - self.logical_validity().into_validity(), + self.logical_validity() + .into_validity(self.dtype().nullability()), ) }; diff --git a/encodings/datetime-parts/src/array.rs b/encodings/datetime-parts/src/array.rs index 5e87631a84..9ee15a1caa 100644 --- a/encodings/datetime-parts/src/array.rs +++ b/encodings/datetime-parts/src/array.rs @@ -103,11 +103,9 @@ impl DateTimePartsArray { } pub fn validity(&self) -> Validity { - if self.dtype().is_nullable() { - self.days().logical_validity().into_validity() - } else { - Validity::NonNullable - } + self.days() + .logical_validity() + .into_validity(self.dtype().nullability()) } } diff --git a/vortex-array/src/array/chunked/canonical.rs b/vortex-array/src/array/chunked/canonical.rs index 33a4c7e2dc..db7a72c726 100644 --- a/vortex-array/src/array/chunked/canonical.rs +++ b/vortex-array/src/array/chunked/canonical.rs @@ -18,11 +18,9 @@ use crate::{ impl IntoCanonical for ChunkedArray { fn into_canonical(self) -> VortexResult { - let validity = if self.dtype().is_nullable() { - self.logical_validity().into_validity() - } else { - Validity::NonNullable - }; + let validity = self + .logical_validity() + .into_validity(self.dtype().nullability()); try_canonicalize_chunks(self.chunks().collect(), validity, self.dtype()) } } diff --git a/vortex-array/src/validity.rs b/vortex-array/src/validity.rs index 71d605fb82..6128796a79 100644 --- a/vortex-array/src/validity.rs +++ b/vortex-array/src/validity.rs @@ -301,6 +301,12 @@ impl Validity { _ => {} }; + let own_nullability = if self == Validity::NonNullable { + Nullability::NonNullable + } else { + Nullability::Nullable + }; + let source = match self { Validity::NonNullable => BoolArray::from(BooleanBuffer::new_set(len)), Validity::AllValid => BoolArray::from(BooleanBuffer::new_set(len)), @@ -317,7 +323,7 @@ impl Validity { let patches = Patches::new(len, indices.clone(), patch_values.into_array()); - Validity::try_from(source.patch(patches)?.into_array()) + Self::from_array(source.patch(patches)?.into_array(), own_nullability) } /// Convert into a nullable variant @@ -327,6 +333,14 @@ impl Validity { _ => self, } } + + /// Create Validity from boolean array with given nullability of the array. + /// + /// Note: You want to pass the nullability of parent array and not the nullability of the validity array itself + /// as that is always nonnullable + pub fn from_array(value: ArrayData, nullability: Nullability) -> VortexResult { + LogicalValidity::try_from(value).map(|a| a.into_validity(nullability)) + } } impl PartialEq for Validity { @@ -371,14 +385,6 @@ impl From for Validity { } } -impl TryFrom for Validity { - type Error = VortexError; - - fn try_from(value: ArrayData) -> Result { - LogicalValidity::try_from(value).map(|a| a.into_validity()) - } -} - impl FromIterator for Validity { fn from_iter>(iter: T) -> Self { let validities: Vec = iter.into_iter().collect(); @@ -489,9 +495,16 @@ impl LogicalValidity { } } - pub fn into_validity(self) -> Validity { + pub fn into_validity(self, nullability: Nullability) -> Validity { + assert!( + nullability == Nullability::Nullable || matches!(self, Self::AllValid(_)), + "NonNullable validity must be AllValid", + ); match self { - Self::AllValid(_) => Validity::AllValid, + Self::AllValid(_) => match nullability { + Nullability::NonNullable => Validity::NonNullable, + Nullability::Nullable => Validity::AllValid, + }, Self::AllInvalid(_) => Validity::AllInvalid, Self::Array(a) => Validity::Array(a), } @@ -533,9 +546,10 @@ impl IntoArrayData for LogicalValidity { mod tests { use rstest::rstest; use vortex_buffer::{buffer, Buffer}; + use vortex_dtype::Nullability; use crate::array::{BoolArray, PrimitiveArray}; - use crate::validity::Validity; + use crate::validity::{LogicalValidity, Validity}; use crate::IntoArrayData; #[rstest] @@ -574,4 +588,17 @@ mod tests { .patch(2, &buffer![4].into_array(), Validity::AllInvalid) .unwrap(); } + + #[test] + #[should_panic] + fn into_validity_nullable() { + LogicalValidity::AllInvalid(10).into_validity(Nullability::NonNullable); + } + + #[test] + #[should_panic] + fn into_validity_nullable_array() { + LogicalValidity::Array(BoolArray::from_iter(vec![true, false]).into_array()) + .into_validity(Nullability::NonNullable); + } }