Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions encodings/parquet-variant/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ fn logical_shredded_from_parquet_field(
let validity = field_struct.validity()?;
// `unmasked_field_by_name_opt` intentionally ignores the parent struct validity.
// Reapply it here so null wrapper rows become null typed/raw rows downstream.
let typed_value = if validity.no_nulls() {
let typed_value = if validity.definitely_no_nulls() {
typed_value.clone()
} else {
typed_value
Expand All @@ -251,7 +251,7 @@ fn logical_shredded_from_parquet_field(
let value = field_struct
.unmasked_field_by_name_opt("value")
.map(|value| {
if validity.no_nulls() {
if validity.definitely_no_nulls() {
Ok(value.clone())
} else {
value.clone().mask(validity.to_array(value.len()))
Expand Down
2 changes: 1 addition & 1 deletion encodings/zstd/src/compute/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl CastReduce for Zstd {
child_to_validity(array.slots()[0].as_ref(), array.dtype().nullability());
let has_nulls = !unsliced_validity
.slice(array.slice_start()..array.slice_stop())?
.no_nulls();
.definitely_no_nulls();

// We don't attempt to handle casting when there are nulls.
if has_nulls {
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/aggregate_fn/fns/all_non_distinct/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(super) fn shared_validity_mask(
) -> VortexResult<Option<Mask>> {
let lhs_validity = lhs.validity()?;
let rhs_validity = rhs.validity()?;
if lhs_validity.no_nulls() && rhs_validity.no_nulls() {
if lhs_validity.definitely_no_nulls() && rhs_validity.definitely_no_nulls() {
return Ok(Some(Mask::new_true(lhs.len())));
}

Expand All @@ -33,7 +33,7 @@ pub(super) fn filter_valid_rows_if_needed(
ctx: &mut ExecutionCtx,
) -> VortexResult<Option<(ArrayRef, ArrayRef)>> {
let validity = lhs.validity()?;
if validity.no_nulls() {
if validity.definitely_no_nulls() {
return Ok(None);
}

Expand Down
2 changes: 1 addition & 1 deletion vortex-array/src/arrays/dict/compute/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl CastReduce for Dict {
fn cast(array: ArrayView<'_, Dict>, dtype: &DType) -> VortexResult<Option<ArrayRef>> {
// Can have un-reference null values making the cast of values fail without a possible mask.
// TODO(joe): optimize this, could look at accessible values and fill_null not those?
if !dtype.is_nullable() && !array.values().validity()?.no_nulls() {
if !dtype.is_nullable() && !array.values().validity()?.definitely_no_nulls() {
return Ok(None);
}
// Cast the dictionary values to the target type
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/arrays/filter/execute/take/fixed_width.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ where
take_filtered_values::<T, P>(&child, filter, ranks, None)?
};

let output_validity = if child_validity.no_nulls() {
let output_validity = if child_validity.definitely_no_nulls() {
ranks_validity.union_nullability(child_validity.nullability())
} else {
let translated_indices =
Expand All @@ -120,7 +120,7 @@ where
AllOr::Some(buf) => {
let taken = take_filtered_values(child.as_slice(), filter, ranks, Some(buf))?;

let output_validity = if child_validity.no_nulls() {
let output_validity = if child_validity.definitely_no_nulls() {
ranks_validity.union_nullability(child_validity.nullability())
} else {
let translated_indices =
Expand Down
21 changes: 19 additions & 2 deletions vortex-array/src/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,30 @@ impl Validity {
}
}

/// Returns `true` if this validity guarantees no null values, i.e. it is either
/// Returns `true` if this validity *definitely* contains no null values, i.e. it is either
/// [`Validity::NonNullable`] or [`Validity::AllValid`].
///
/// Returning `false` does not prove the presence of nulls: a [`Validity::Array`] may still
/// resolve to all-valid once executed. Callers must treat `false` as "unknown without
/// compute" and either fall back to a null-handling path or execute the validity.
#[inline]
pub fn no_nulls(&self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_no_nulls?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an approximate function. Do you think has_no_nulls says that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They all sound vague to me, does it really need to be a function vs allowing callers to just have it explicitly there?

pub fn definitely_no_nulls(&self) -> bool {
matches!(self, Self::NonNullable | Self::AllValid)
}

/// Returns whether this validity contains no null values, executing the validity array if
/// necessary.
///
/// This is the exact counterpart to [`Self::definitely_no_nulls`]: use it when the caller
/// needs a definitive answer rather than a cheap, conservative one.
pub fn execute_no_nulls(&self, length: usize, ctx: &mut ExecutionCtx) -> VortexResult<bool> {
match self {
Self::NonNullable | Self::AllValid => Ok(true),
Self::AllInvalid => Ok(length == 0),
Self::Array(_) => Ok(self.execute_mask(length, ctx)?.all_true()),
}
}

/// The union nullability and validity.
#[inline]
pub fn union_nullability(self, nullability: Nullability) -> Self {
Expand Down
2 changes: 1 addition & 1 deletion vortex-cuda/src/arrow/list_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ async fn rebuild_primitive_list_view_child(
} = elements.into_data_parts();

vortex_ensure!(
validity.no_nulls(),
validity.execute_no_nulls(elements_len, ctx.execution_ctx())?,
"cannot export non-contiguous device-resident ListViewArray with nullable {child_name}: GPU child validity rebuild is not implemented"
);

Expand Down
Loading