From b58f1d5f971a773f27c2a003082d3ab493a9d89f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 27 May 2026 10:46:05 +0000 Subject: [PATCH 1/6] Add MaskNullAsFalse execution target and make Mask strict Mask execution now requires a non-nullable boolean array and errors on nullable input. The new MaskNullAsFalse executable preserves the previous null-as-false coercion for filter and pruning predicates over nullable data, where SQL semantics treat NULL as not matching. Predicate-evaluation call sites (filter, prune, dict filter, is_constant) use MaskNullAsFalse; validity-array call sites keep the stricter Mask. Signed-off-by: Claude --- vortex-array/public-api.lock | 18 +++ .../src/aggregate_fn/fns/is_constant/mod.rs | 4 +- vortex-array/src/mask.rs | 107 ++++++++++++++---- vortex-cuda/src/layout.rs | 5 +- vortex-layout/src/layouts/dict/reader.rs | 6 +- vortex-layout/src/layouts/flat/reader.rs | 5 +- vortex-layout/src/layouts/partitioned.rs | 7 +- vortex-layout/src/layouts/row_idx/mod.rs | 6 +- vortex-layout/src/layouts/zoned/zone_map.rs | 7 +- vortex/public-api.lock | 2 + vortex/src/lib.rs | 1 + 11 files changed, 131 insertions(+), 37 deletions(-) diff --git a/vortex-array/public-api.lock b/vortex-array/public-api.lock index 9525aef3a88..82384e2e9fe 100644 --- a/vortex-array/public-api.lock +++ b/vortex-array/public-api.lock @@ -14734,6 +14734,20 @@ pub fn vortex_array::scalar_fn::fns::zip::ZipExecuteAdaptor::execute_parent(& pub mod vortex_array::mask +pub struct vortex_array::mask::MaskNullAsFalse(_) + +impl vortex_array::mask::MaskNullAsFalse + +pub fn vortex_array::mask::MaskNullAsFalse::into_mask(self) -> vortex_mask::Mask + +impl core::convert::From for vortex_mask::Mask + +pub fn vortex_mask::Mask::from(vortex_array::mask::MaskNullAsFalse) -> Self + +impl vortex_array::Executable for vortex_array::mask::MaskNullAsFalse + +pub fn vortex_array::mask::MaskNullAsFalse::execute(vortex_array::ArrayRef, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult + pub mod vortex_array::matcher pub struct vortex_array::matcher::AnyArray @@ -26326,6 +26340,10 @@ impl vortex_array::Executable for vortex_array::arrays::null::NullArray pub fn vortex_array::arrays::null::NullArray::execute(vortex_array::ArrayRef, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +impl vortex_array::Executable for vortex_array::mask::MaskNullAsFalse + +pub fn vortex_array::mask::MaskNullAsFalse::execute(vortex_array::ArrayRef, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult + impl vortex_array::Executable for vortex_buffer::bit::buf::BitBuffer pub fn vortex_buffer::bit::buf::BitBuffer::execute(vortex_array::ArrayRef, &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult diff --git a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs index b14b2a050f8..9c3a6278f02 100644 --- a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs @@ -13,7 +13,6 @@ mod varbin; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; -use vortex_mask::Mask; use self::bool::check_bool_constant; use self::decimal::check_decimal_constant; @@ -44,6 +43,7 @@ use crate::expr::stats::Precision; use crate::expr::stats::Stat; use crate::expr::stats::StatsProvider; use crate::expr::stats::StatsProviderExt; +use crate::mask::MaskNullAsFalse; use crate::scalar::Scalar; use crate::scalar_fn::fns::operators::Operator; @@ -74,7 +74,7 @@ fn arrays_value_equal(a: &ArrayRef, b: &ArrayRef, ctx: &mut ExecutionCtx) -> Vor // Compare values element-wise. Result is null where both inputs are null, // true/false where both are valid. let eq_result = a.binary(b.clone(), Operator::Eq)?; - let eq_result = eq_result.execute::(ctx)?; + let eq_result = eq_result.execute::(ctx)?.into_mask(); Ok(eq_result.true_count() == valid_count) } diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index ab035670e24..fb04606c2b9 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -17,33 +17,90 @@ use crate::columnar::Columnar; use crate::dtype::DType; impl Executable for Mask { + /// Executes a boolean array into a [`Mask`]. + /// + /// The array must have a non-nullable boolean dtype. Use [`MaskNullAsFalse`] to execute a + /// nullable boolean array, coercing null elements to `false`. fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { - if !matches!(array.dtype(), DType::Bool(_)) { - vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); - } + execute_mask(array, ctx, NullHandling::Reject) + } +} - if let Some(constant) = array.as_opt::() { - let mask_value = constant.scalar().as_bool().value().unwrap_or(false); - return Ok(Mask::new(array.len(), mask_value)); - } +/// An [`Executable`] target that executes a boolean array into a [`Mask`], coercing null +/// elements to `false`. +/// +/// [`Mask`] itself requires a non-nullable boolean array and errors on nullable input. Use this +/// wrapper for filter and pruning predicates over nullable data, where SQL semantics treat +/// `NULL` as not matching. +pub struct MaskNullAsFalse(Mask); - let array_len = array.len(); - Ok(match array.execute(ctx)? { - Columnar::Constant(s) => { - Mask::new(array_len, s.scalar().as_bool().value().unwrap_or(false)) - } - Columnar::Canonical(a) => { - let bool = a.into_array().execute::(ctx)?; - let mask = bool - .as_ref() - .validity()? - .execute_mask(bool.as_ref().len(), ctx)?; - let bits = bool.into_bit_buffer(); - // To handle nullable boolean arrays, we treat nulls as false in the mask. - // TODO(ngates): is this correct? Feels like we should just force the caller to - // pass non-nullable boolean arrays. - mask.bitand(&Mask::from(bits)) - } - }) +impl MaskNullAsFalse { + /// Consumes the wrapper and returns the underlying [`Mask`]. + pub fn into_mask(self) -> Mask { + self.0 + } +} + +impl From for Mask { + fn from(value: MaskNullAsFalse) -> Self { + value.0 + } +} + +impl Executable for MaskNullAsFalse { + fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { + execute_mask(array, ctx, NullHandling::AsFalse).map(Self) + } +} + +/// How [`execute_mask`] treats null elements of a nullable boolean array. +enum NullHandling { + /// Error if the boolean array is nullable. + Reject, + /// Treat null elements as `false`. + AsFalse, +} + +fn execute_mask( + array: ArrayRef, + ctx: &mut ExecutionCtx, + null_handling: NullHandling, +) -> VortexResult { + if !matches!(array.dtype(), DType::Bool(_)) { + vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); } + + if let Some(constant) = array.as_opt::() { + let mask_value = constant.scalar().as_bool().value().unwrap_or(false); + return Ok(Mask::new(array.len(), mask_value)); + } + + let array_len = array.len(); + Ok(match array.execute(ctx)? { + Columnar::Constant(s) => { + Mask::new(array_len, s.scalar().as_bool().value().unwrap_or(false)) + } + Columnar::Canonical(a) => { + let bool = a.into_array().execute::(ctx)?; + match null_handling { + NullHandling::Reject => { + if bool.as_ref().dtype().is_nullable() { + vortex_bail!( + "Mask requires a non-nullable boolean array, not {}; \ + use MaskNullAsFalse to coerce nulls to false", + bool.as_ref().dtype() + ); + } + Mask::from(bool.into_bit_buffer()) + } + NullHandling::AsFalse => { + let validity = bool + .as_ref() + .validity()? + .execute_mask(bool.as_ref().len(), ctx)?; + validity.bitand(&Mask::from(bool.into_bit_buffer())) + } + } + } + }) } diff --git a/vortex-cuda/src/layout.rs b/vortex-cuda/src/layout.rs index 76e5a5ba5fc..da0fa28931d 100644 --- a/vortex-cuda/src/layout.rs +++ b/vortex-cuda/src/layout.rs @@ -59,6 +59,7 @@ use vortex::layout::sequence::SendableSequentialStream; use vortex::layout::sequence::SequencePointer; use vortex::layout::vtable; use vortex::mask::Mask; +use vortex::mask::MaskNullAsFalse; use vortex::scalar::Scalar; use vortex::scalar::ScalarTruncation; use vortex::scalar::lower_bound; @@ -329,12 +330,12 @@ impl LayoutReader for CudaFlatReader { let array = array.apply(&expr)?; let array = array.filter(mask.clone())?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?; + let array_mask = array.execute::(&mut ctx)?.into_mask(); mask.intersect_by_rank(&array_mask) } else { let array = array.apply(&expr)?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?; + let array_mask = array.execute::(&mut ctx)?.into_mask(); mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/dict/reader.rs b/vortex-layout/src/layouts/dict/reader.rs index 96f12d53ece..19e002b42ee 100644 --- a/vortex-layout/src/layouts/dict/reader.rs +++ b/vortex-layout/src/layouts/dict/reader.rs @@ -21,6 +21,7 @@ use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; use vortex_array::expr::root; +use vortex_array::mask::MaskNullAsFalse; use vortex_array::optimizer::ArrayOptimizer; use vortex_error::VortexError; use vortex_error::VortexExpect; @@ -212,7 +213,10 @@ impl LayoutReader for DictReader { let mask = mask.await?; let mut ctx = session.create_execution_ctx(); - let dict_mask = values.take(codes)?.execute::(&mut ctx)?; + let dict_mask = values + .take(codes)? + .execute::(&mut ctx)? + .into_mask(); Ok(mask.bitand(&dict_mask)) })) diff --git a/vortex-layout/src/layouts/flat/reader.rs b/vortex-layout/src/layouts/flat/reader.rs index 8817fe5c678..6b4e7b0c240 100644 --- a/vortex-layout/src/layouts/flat/reader.rs +++ b/vortex-layout/src/layouts/flat/reader.rs @@ -15,6 +15,7 @@ use vortex_array::VortexSessionExecute; use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; +use vortex_array::mask::MaskNullAsFalse; use vortex_array::serde::SerializedArray; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -155,14 +156,14 @@ impl LayoutReader for FlatReader { let array = array.apply(&expr)?; let array = array.filter(mask.clone())?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?; + let array_mask = array.execute::(&mut ctx)?.into_mask(); mask.intersect_by_rank(&array_mask) } else { // Run over the full array, with a simpler bitand at the end. let array = array.apply(&expr)?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?; + let array_mask = array.execute::(&mut ctx)?.into_mask(); mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/partitioned.rs b/vortex-layout/src/layouts/partitioned.rs index d5ac3d44c55..deaccd92513 100644 --- a/vortex-layout/src/layouts/partitioned.rs +++ b/vortex-layout/src/layouts/partitioned.rs @@ -15,10 +15,10 @@ use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::expr::Expression; use vortex_array::expr::transform::PartitionedExpr; +use vortex_array::mask::MaskNullAsFalse; use vortex_array::validity::Validity; use vortex_error::VortexError; use vortex_error::VortexResult; -use vortex_mask::Mask; use vortex_session::VortexSession; use crate::ArrayFuture; @@ -90,7 +90,10 @@ impl PartitionedExprEval

for PartitionedExpr

{ .into_array(); let mut ctx = session.create_execution_ctx(); - let root_mask = root_scope.apply(&self.root)?.execute::(&mut ctx)?; + let root_mask = root_scope + .apply(&self.root)? + .execute::(&mut ctx)? + .into_mask(); let mask = mask.bitand(&root_mask); diff --git a/vortex-layout/src/layouts/row_idx/mod.rs b/vortex-layout/src/layouts/row_idx/mod.rs index 18fc62b05cd..5693ae7dc38 100644 --- a/vortex-layout/src/layouts/row_idx/mod.rs +++ b/vortex-layout/src/layouts/row_idx/mod.rs @@ -32,6 +32,7 @@ use vortex_array::expr::root; use vortex_array::expr::transform::PartitionedExpr; use vortex_array::expr::transform::partition; use vortex_array::expr::transform::replace; +use vortex_array::mask::MaskNullAsFalse; use vortex_array::scalar::PValue; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -295,7 +296,10 @@ fn row_idx_mask_future( let array = idx_array(row_offset, &row_range).into_array(); let mut ctx = session.create_execution_ctx(); - let result_mask = array.apply(&expr)?.execute::(&mut ctx)?; + let result_mask = array + .apply(&expr)? + .execute::(&mut ctx)? + .into_mask(); Ok(result_mask.bitand(&mask.await?)) }) diff --git a/vortex-layout/src/layouts/zoned/zone_map.rs b/vortex-layout/src/layouts/zoned/zone_map.rs index 16360ff287d..69b98d5e41a 100644 --- a/vortex-layout/src/layouts/zoned/zone_map.rs +++ b/vortex-layout/src/layouts/zoned/zone_map.rs @@ -14,6 +14,7 @@ use vortex_array::arrays::StructArray; use vortex_array::dtype::DType; use vortex_array::expr::Expression; use vortex_array::expr::stats::Stat; +use vortex_array::mask::MaskNullAsFalse; use vortex_array::scalar_fn::internal::row_count::contains_row_count; use vortex_array::scalar_fn::internal::row_count::substitute_row_count; use vortex_array::validity::Validity; @@ -99,12 +100,14 @@ impl ZoneMap { let applied = self.array.clone().into_array().apply(predicate)?; if num_zones == 0 || !contains_row_count(&applied) { - return applied.execute::(&mut ctx); + return Ok(applied.execute::(&mut ctx)?.into_mask()); } let row_count_array = row_count_array(self.zone_len, self.row_count, num_zones)?; let substituted = substitute_row_count(applied, &row_count_array)?; - substituted.execute::(&mut ctx) + Ok(substituted + .execute::(&mut ctx)? + .into_mask()) } } diff --git a/vortex/public-api.lock b/vortex/public-api.lock index 7be026902db..4a5f1d4fe75 100644 --- a/vortex/public-api.lock +++ b/vortex/public-api.lock @@ -112,6 +112,8 @@ pub mod vortex::mask pub use vortex::mask::<> +pub use vortex::mask::MaskNullAsFalse + pub mod vortex::metrics pub use vortex::metrics::<> diff --git a/vortex/src/lib.rs b/vortex/src/lib.rs index 8668de339cb..b5241797a01 100644 --- a/vortex/src/lib.rs +++ b/vortex/src/lib.rs @@ -76,6 +76,7 @@ pub mod layout { } pub mod mask { + pub use vortex_array::mask::MaskNullAsFalse; pub use vortex_mask::*; } From f1d5313e23884cad242322ffb1b3cecded761b81 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 27 May 2026 11:04:01 +0000 Subject: [PATCH 2/6] Inline Mask and MaskNullAsFalse execution impls Remove the shared NullHandling enum and execute_mask helper in favor of a self-contained Executable impl for each target. Mask still requires a non-nullable boolean array; MaskNullAsFalse coerces nulls to false. Signed-off-by: Claude --- vortex-array/src/mask.rs | 99 +++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 52 deletions(-) diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index fb04606c2b9..3185a607315 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -22,7 +22,32 @@ impl Executable for Mask { /// The array must have a non-nullable boolean dtype. Use [`MaskNullAsFalse`] to execute a /// nullable boolean array, coercing null elements to `false`. fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { - execute_mask(array, ctx, NullHandling::Reject) + if !matches!(array.dtype(), DType::Bool(_)) { + vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); + } + + if let Some(constant) = array.as_opt::() { + let mask_value = constant.scalar().as_bool().value().unwrap_or(false); + return Ok(Mask::new(array.len(), mask_value)); + } + + let array_len = array.len(); + Ok(match array.execute(ctx)? { + Columnar::Constant(s) => { + Mask::new(array_len, s.scalar().as_bool().value().unwrap_or(false)) + } + Columnar::Canonical(a) => { + let bool = a.into_array().execute::(ctx)?; + if bool.as_ref().dtype().is_nullable() { + vortex_bail!( + "Mask requires a non-nullable boolean array, not {}; \ + use MaskNullAsFalse to coerce nulls to false", + bool.as_ref().dtype() + ); + } + Mask::from(bool.into_bit_buffer()) + } + }) } } @@ -49,58 +74,28 @@ impl From for Mask { impl Executable for MaskNullAsFalse { fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { - execute_mask(array, ctx, NullHandling::AsFalse).map(Self) - } -} - -/// How [`execute_mask`] treats null elements of a nullable boolean array. -enum NullHandling { - /// Error if the boolean array is nullable. - Reject, - /// Treat null elements as `false`. - AsFalse, -} - -fn execute_mask( - array: ArrayRef, - ctx: &mut ExecutionCtx, - null_handling: NullHandling, -) -> VortexResult { - if !matches!(array.dtype(), DType::Bool(_)) { - vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); - } - - if let Some(constant) = array.as_opt::() { - let mask_value = constant.scalar().as_bool().value().unwrap_or(false); - return Ok(Mask::new(array.len(), mask_value)); - } + if !matches!(array.dtype(), DType::Bool(_)) { + vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); + } - let array_len = array.len(); - Ok(match array.execute(ctx)? { - Columnar::Constant(s) => { - Mask::new(array_len, s.scalar().as_bool().value().unwrap_or(false)) + if let Some(constant) = array.as_opt::() { + let mask_value = constant.scalar().as_bool().value().unwrap_or(false); + return Ok(Self(Mask::new(array.len(), mask_value))); } - Columnar::Canonical(a) => { - let bool = a.into_array().execute::(ctx)?; - match null_handling { - NullHandling::Reject => { - if bool.as_ref().dtype().is_nullable() { - vortex_bail!( - "Mask requires a non-nullable boolean array, not {}; \ - use MaskNullAsFalse to coerce nulls to false", - bool.as_ref().dtype() - ); - } - Mask::from(bool.into_bit_buffer()) - } - NullHandling::AsFalse => { - let validity = bool - .as_ref() - .validity()? - .execute_mask(bool.as_ref().len(), ctx)?; - validity.bitand(&Mask::from(bool.into_bit_buffer())) - } + + let array_len = array.len(); + Ok(Self(match array.execute(ctx)? { + Columnar::Constant(s) => { + Mask::new(array_len, s.scalar().as_bool().value().unwrap_or(false)) } - } - }) + Columnar::Canonical(a) => { + let bool = a.into_array().execute::(ctx)?; + let validity = bool + .as_ref() + .validity()? + .execute_mask(bool.as_ref().len(), ctx)?; + validity.bitand(&Mask::from(bool.into_bit_buffer())) + } + })) + } } From 587308b4c90f09543bbc3c8ce95d9b3e39752eb7 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Wed, 27 May 2026 13:01:13 +0100 Subject: [PATCH 3/6] fix Signed-off-by: Joe Isaacs --- vortex-array/src/mask.rs | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 3185a607315..7fb25d4d627 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -12,9 +12,9 @@ use crate::Executable; use crate::ExecutionCtx; use crate::IntoArray; use crate::arrays::BoolArray; -use crate::arrays::Constant; use crate::columnar::Columnar; use crate::dtype::DType; +use crate::dtype::Nullability; impl Executable for Mask { /// Executes a boolean array into a [`Mask`]. @@ -22,13 +22,11 @@ impl Executable for Mask { /// The array must have a non-nullable boolean dtype. Use [`MaskNullAsFalse`] to execute a /// nullable boolean array, coercing null elements to `false`. fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { - if !matches!(array.dtype(), DType::Bool(_)) { - vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); - } - - if let Some(constant) = array.as_opt::() { - let mask_value = constant.scalar().as_bool().value().unwrap_or(false); - return Ok(Mask::new(array.len(), mask_value)); + if !matches!(array.dtype(), DType::Bool(Nullability::NonNullable)) { + vortex_bail!( + "Mask array must have boolean(NonNullable) dtype, not {}", + array.dtype() + ); } let array_len = array.len(); @@ -38,13 +36,6 @@ impl Executable for Mask { } Columnar::Canonical(a) => { let bool = a.into_array().execute::(ctx)?; - if bool.as_ref().dtype().is_nullable() { - vortex_bail!( - "Mask requires a non-nullable boolean array, not {}; \ - use MaskNullAsFalse to coerce nulls to false", - bool.as_ref().dtype() - ); - } Mask::from(bool.into_bit_buffer()) } }) @@ -78,11 +69,6 @@ impl Executable for MaskNullAsFalse { vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); } - if let Some(constant) = array.as_opt::() { - let mask_value = constant.scalar().as_bool().value().unwrap_or(false); - return Ok(Self(Mask::new(array.len(), mask_value))); - } - let array_len = array.len(); Ok(Self(match array.execute(ctx)? { Columnar::Constant(s) => { From 7c58c576f011aaa3ef1982ad5402326756c28017 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Fri, 12 Jun 2026 15:25:44 +0100 Subject: [PATCH 4/6] fix use Signed-off-by: Joe Isaacs --- .../src/aggregate_fn/fns/is_constant/mod.rs | 5 +- vortex-array/src/executor.rs | 32 +++- vortex-array/src/mask.rs | 142 +++++++++++++----- vortex-buffer/src/bit/buf.rs | 54 +++++++ vortex-buffer/src/bit/ops.rs | 42 ++++++ vortex-cuda/src/layout.rs | 6 +- vortex-layout/src/layouts/dict/reader.rs | 5 +- vortex-layout/src/layouts/flat/reader.rs | 6 +- vortex-layout/src/layouts/partitioned.rs | 6 +- vortex-layout/src/layouts/row_idx/mod.rs | 5 +- vortex-layout/src/layouts/zoned/zone_map.rs | 7 +- vortex/src/lib.rs | 2 +- 12 files changed, 249 insertions(+), 63 deletions(-) diff --git a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs index 9c3a6278f02..18d4165fcc5 100644 --- a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs @@ -13,6 +13,7 @@ mod varbin; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; +use vortex_mask::Mask; use self::bool::check_bool_constant; use self::decimal::check_decimal_constant; @@ -26,6 +27,7 @@ use crate::ArrayRef; use crate::Canonical; use crate::Columnar; use crate::ExecutionCtx; +use crate::Executor; use crate::IntoArray; use crate::aggregate_fn::Accumulator; use crate::aggregate_fn::AggregateFnId; @@ -43,7 +45,6 @@ use crate::expr::stats::Precision; use crate::expr::stats::Stat; use crate::expr::stats::StatsProvider; use crate::expr::stats::StatsProviderExt; -use crate::mask::MaskNullAsFalse; use crate::scalar::Scalar; use crate::scalar_fn::fns::operators::Operator; @@ -74,7 +75,7 @@ fn arrays_value_equal(a: &ArrayRef, b: &ArrayRef, ctx: &mut ExecutionCtx) -> Vor // Compare values element-wise. Result is null where both inputs are null, // true/false where both are valid. let eq_result = a.binary(b.clone(), Operator::Eq)?; - let eq_result = eq_result.execute::(ctx)?.into_mask(); + let eq_result = eq_result.null_as_false().execute::(ctx)?; Ok(eq_result.true_count() == valid_count) } diff --git a/vortex-array/src/executor.rs b/vortex-array/src/executor.rs index d6070ac1a4d..a102456e765 100644 --- a/vortex-array/src/executor.rs +++ b/vortex-array/src/executor.rs @@ -62,15 +62,33 @@ pub(crate) fn max_iterations() -> usize { *MAX_ITERATIONS } -/// Marker trait for types that an [`ArrayRef`] can be executed into. +/// Trait for types that a `Source` can be executed into. /// -/// Implementors must provide an implementation of `execute` that takes -/// an [`ArrayRef`] and an [`ExecutionCtx`], and produces an instance of the -/// implementor type. +/// `Source` defaults to [`ArrayRef`], so `impl Executable for X` defines how to execute an +/// array into an `X`. Other sources are lightweight stack-allocated [`Executor`] adapters +/// that adjust execution semantics without wrapping the array in another array node, e.g. +/// `impl Executable for Mask` executes a nullable boolean array into a +/// [`vortex_mask::Mask`], coercing nulls to `false`. /// -/// Users should use the `Array::execute` or `Array::execute_as` methods -pub trait Executable: Sized { - fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult; +/// Users should use the [`Executor::execute`] method (or the inherent `ArrayRef::execute` +/// and `ArrayRef::execute_as` methods) rather than calling this trait directly. +pub trait Executable: Sized { + fn execute(source: Source, ctx: &mut ExecutionCtx) -> VortexResult; +} + +/// Types that can drive execution into an [`Executable`] target. +/// +/// Implemented by [`ArrayRef`] and by stack-allocated adapters such as +/// [`NullAsFalse`](crate::mask::NullAsFalse): +/// +/// ```ignore +/// let mask = array.null_as_false().execute::(ctx)?; +/// ``` +pub trait Executor: Sized { + /// Execute into an instance of `E`. + fn execute>(self, ctx: &mut ExecutionCtx) -> VortexResult { + E::execute(self, ctx) + } } #[expect(clippy::same_name_method)] diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 7fb25d4d627..13f79161d93 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -1,8 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -use std::ops::BitAnd; - use vortex_error::VortexResult; use vortex_error::vortex_bail; use vortex_mask::Mask; @@ -10,17 +8,20 @@ use vortex_mask::Mask; use crate::ArrayRef; use crate::Executable; use crate::ExecutionCtx; +use crate::Executor; use crate::IntoArray; use crate::arrays::BoolArray; use crate::columnar::Columnar; use crate::dtype::DType; use crate::dtype::Nullability; +use crate::validity::Validity; impl Executable for Mask { /// Executes a boolean array into a [`Mask`]. /// - /// The array must have a non-nullable boolean dtype. Use [`MaskNullAsFalse`] to execute a - /// nullable boolean array, coercing null elements to `false`. + /// The array must have a non-nullable boolean dtype. Use + /// [`ArrayRef::null_as_false`] to execute a nullable boolean array, coercing null + /// elements to `false`. fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { if !matches!(array.dtype(), DType::Bool(Nullability::NonNullable)) { vortex_bail!( @@ -42,46 +43,119 @@ impl Executable for Mask { } } -/// An [`Executable`] target that executes a boolean array into a [`Mask`], coercing null -/// elements to `false`. +/// An [`Executor`] adapter that coerces null elements of a boolean array to `false` before +/// executing into the target type. /// -/// [`Mask`] itself requires a non-nullable boolean array and errors on nullable input. Use this -/// wrapper for filter and pruning predicates over nullable data, where SQL semantics treat -/// `NULL` as not matching. -pub struct MaskNullAsFalse(Mask); - -impl MaskNullAsFalse { - /// Consumes the wrapper and returns the underlying [`Mask`]. - pub fn into_mask(self) -> Mask { - self.0 - } -} +/// Created by [`ArrayRef::null_as_false`]. The adapter lives on the stack and moves the +/// underlying [`ArrayRef`], so it does not allocate a wrapper array node. +/// +/// Use for filter and pruning predicates over nullable data, where SQL semantics treat +/// `NULL` as not matching: +/// +/// ```ignore +/// let mask = array.null_as_false().execute::(ctx)?; +/// ``` +pub struct NullAsFalse(ArrayRef); -impl From for Mask { - fn from(value: MaskNullAsFalse) -> Self { - value.0 +impl ArrayRef { + /// Returns an [`Executor`] that treats null elements of this boolean array as `false`. + pub fn null_as_false(self) -> NullAsFalse { + NullAsFalse(self) } } -impl Executable for MaskNullAsFalse { - fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { +impl Executor for NullAsFalse {} + +impl Executable for Mask { + /// Executes a boolean array into a [`Mask`], coercing null elements to `false`. + /// + /// The validity is folded into the value bits with a single AND that reuses the value + /// buffer when it is uniquely owned. + fn execute(source: NullAsFalse, ctx: &mut ExecutionCtx) -> VortexResult { + let array = source.0; if !matches!(array.dtype(), DType::Bool(_)) { vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); } + if !array.dtype().is_nullable() { + return array.execute(ctx); + } - let array_len = array.len(); - Ok(Self(match array.execute(ctx)? { - Columnar::Constant(s) => { - Mask::new(array_len, s.scalar().as_bool().value().unwrap_or(false)) + let len = array.len(); + Ok(match array.execute::(ctx)? { + Columnar::Constant(c) => { + Mask::new(len, c.scalar().as_bool().value().unwrap_or(false)) } - Columnar::Canonical(a) => { - let bool = a.into_array().execute::(ctx)?; - let validity = bool - .as_ref() - .validity()? - .execute_mask(bool.as_ref().len(), ctx)?; - validity.bitand(&Mask::from(bool.into_bit_buffer())) + Columnar::Canonical(c) => { + let bool = c.into_array().execute::(ctx)?; + match bool.as_ref().validity()? { + Validity::NonNullable | Validity::AllValid => { + Mask::from_buffer(bool.into_bit_buffer()) + } + Validity::AllInvalid => Mask::new_false(len), + Validity::Array(v) => { + let validity_bits = v.execute::(ctx)?.into_bit_buffer(); + Mask::from_buffer(bool.into_bit_buffer().bitand_in_place(&validity_bits)) + } + } } - })) + }) + } +} + +#[cfg(test)] +mod tests { + use vortex_error::VortexResult; + use vortex_mask::Mask; + + use crate::ExecutionCtx; + use crate::Executor; + use crate::IntoArray; + use crate::LEGACY_SESSION; + use crate::VortexSessionExecute; + use crate::arrays::BoolArray; + use crate::arrays::ConstantArray; + use crate::dtype::DType; + use crate::dtype::Nullability; + use crate::scalar::Scalar; + + fn ctx() -> ExecutionCtx { + LEGACY_SESSION.create_execution_ctx() + } + + #[test] + fn null_as_false_non_nullable() -> VortexResult<()> { + let array = BoolArray::from_iter([true, false, true]).into_array(); + let mask = array.null_as_false().execute::(&mut ctx())?; + assert_eq!(mask, Mask::from_iter([true, false, true])); + Ok(()) + } + + #[test] + fn null_as_false_coerces_nulls() -> VortexResult<()> { + let array = BoolArray::from_iter([Some(true), None, Some(false), None]).into_array(); + let mask = array.null_as_false().execute::(&mut ctx())?; + assert_eq!(mask, Mask::from_iter([true, false, false, false])); + Ok(()) + } + + #[test] + fn null_as_false_null_constant() -> VortexResult<()> { + let array = + ConstantArray::new(Scalar::null(DType::Bool(Nullability::Nullable)), 4).into_array(); + let mask = array.null_as_false().execute::(&mut ctx())?; + assert_eq!(mask, Mask::new_false(4)); + Ok(()) + } + + #[test] + fn null_as_false_rejects_non_bool() { + let array = ConstantArray::new(42i32, 4).into_array(); + assert!(array.null_as_false().execute::(&mut ctx()).is_err()); + } + + #[test] + fn mask_rejects_nullable() { + let array = BoolArray::from_iter([Some(true), None]).into_array(); + assert!(array.execute::(&mut ctx()).is_err()); } } diff --git a/vortex-buffer/src/bit/buf.rs b/vortex-buffer/src/bit/buf.rs index 83b28e24b4a..c98bde74594 100644 --- a/vortex-buffer/src/bit/buf.rs +++ b/vortex-buffer/src/bit/buf.rs @@ -25,6 +25,7 @@ use crate::bit::collect_bool_word; use crate::bit::count_ones::count_ones; use crate::bit::get_bit_unchecked; use crate::bit::ops::bitwise_binary_op; +use crate::bit::ops::bitwise_binary_op_mut; use crate::bit::ops::bitwise_unary_op; use crate::bit::select::bit_select; use crate::buffer; @@ -505,6 +506,27 @@ impl BitBuffer { bitwise_binary_op(self, rhs, |a, b| a & !b) } + /// Compute `self & rhs`, reusing `self`'s allocation when it is uniquely owned and + /// byte-aligned. + /// + /// Falls back to an allocating AND when `self` is shared or its offset is not a multiple + /// of 8. `rhs` may have any offset. + pub fn bitand_in_place(self, rhs: &BitBuffer) -> BitBuffer { + assert_eq!(self.len(), rhs.len(), "Buffers must have the same length"); + + if !self.offset().is_multiple_of(8) { + return &self & rhs; + } + + match self.try_into_mut() { + Ok(mut lhs) => { + bitwise_binary_op_mut(&mut lhs, rhs, |a, b| a & b); + lhs.freeze() + } + Err(this) => &this & rhs, + } + } + /// Iterate through bits in a buffer. /// /// # Arguments @@ -584,6 +606,38 @@ mod tests { use crate::bit::BitBuffer; use crate::buffer; + #[rstest] + #[case(0, 0)] // aligned lhs, aligned rhs + #[case(0, 3)] // aligned lhs, misaligned rhs + #[case(5, 0)] // misaligned lhs (falls back to allocating path) + #[case(11, 7)] // both misaligned + fn test_bitand_in_place(#[case] lhs_offset: usize, #[case] rhs_offset: usize) { + let len = 200; + let lhs = BitBuffer::from_iter((0..len + lhs_offset).map(|i| i % 2 == 0)) + .slice(lhs_offset..len + lhs_offset); + let rhs = BitBuffer::from_iter((0..len + rhs_offset).map(|i| i % 3 == 0)) + .slice(rhs_offset..len + rhs_offset); + + let expected = &lhs & &rhs; + let actual = lhs.bitand_in_place(&rhs); + + assert_eq!(actual.len(), expected.len()); + assert!(actual.iter().eq(expected.iter())); + } + + #[test] + fn test_bitand_in_place_shared_falls_back() { + let lhs = BitBuffer::from_iter((0..100).map(|i| i % 2 == 0)); + let shared = lhs.clone(); + let rhs = BitBuffer::from_iter((0..100).map(|i| i % 5 == 0)); + + let actual = lhs.bitand_in_place(&rhs); + let expected = &shared & &rhs; + assert!(actual.iter().eq(expected.iter())); + // The shared copy must be unmodified. + assert!(shared.iter().eq((0..100).map(|i| i % 2 == 0))); + } + #[test] fn test_bool() { // Create a new Buffer of length 1024 where the 8th bit is set. diff --git a/vortex-buffer/src/bit/ops.rs b/vortex-buffer/src/bit/ops.rs index 9406c728f06..6d26d93a71b 100644 --- a/vortex-buffer/src/bit/ops.rs +++ b/vortex-buffer/src/bit/ops.rs @@ -87,6 +87,48 @@ pub(super) fn bitwise_unary_op_mut u64>(buffer: &mut BitBufferM } } +/// Applies `op` word-wise to `lhs` in place, zipping with the logical words of `rhs`. +/// +/// `lhs` must be byte-aligned (`offset % 8 == 0`) and have the same length as `rhs`. Bits +/// beyond `lhs.len()` in the final byte are left unspecified after this call. +pub(super) fn bitwise_binary_op_mut u64>( + lhs: &mut BitBufferMut, + rhs: &BitBuffer, + mut op: F, +) { + assert_eq!(lhs.len(), rhs.len(), "Buffers must have the same length"); + assert!( + lhs.offset().is_multiple_of(8), + "lhs must be byte-aligned for in-place bitwise ops" + ); + + let byte_start = lhs.offset() / 8; + let byte_len = lhs.len().div_ceil(8); + let slice = &mut lhs.as_mut_slice()[byte_start..byte_start + byte_len]; + + // `iter_padded` yields exactly `len.div_ceil(64)` logical words, which matches the number + // of words consumed below: `byte_len / 8` full words plus one for any remainder bytes. + let mut rhs_words = rhs.chunks().iter_padded(); + + let mut chunks = slice.chunks_exact_mut(8); + for chunk in &mut chunks { + let rhs_word = rhs_words.next().unwrap_or(0); + let mut bytes = [0u8; 8]; + bytes.copy_from_slice(chunk); + let value = op(u64::from_le_bytes(bytes), rhs_word); + chunk.copy_from_slice(&value.to_le_bytes()); + } + + let remainder = chunks.into_remainder(); + if !remainder.is_empty() { + let rhs_word = rhs_words.next().unwrap_or(0); + let mut bytes = [0u8; 8]; + bytes[..remainder.len()].copy_from_slice(remainder); + let value = op(u64::from_le_bytes(bytes), rhs_word); + remainder.copy_from_slice(&value.to_le_bytes()[..remainder.len()]); + } +} + pub(super) fn bitwise_binary_op u64>( left: &BitBuffer, right: &BitBuffer, diff --git a/vortex-cuda/src/layout.rs b/vortex-cuda/src/layout.rs index da0fa28931d..d19ffa82c16 100644 --- a/vortex-cuda/src/layout.rs +++ b/vortex-cuda/src/layout.rs @@ -58,8 +58,8 @@ use vortex::layout::segments::SegmentSource; use vortex::layout::sequence::SendableSequentialStream; use vortex::layout::sequence::SequencePointer; use vortex::layout::vtable; +use vortex::array::Executor; use vortex::mask::Mask; -use vortex::mask::MaskNullAsFalse; use vortex::scalar::Scalar; use vortex::scalar::ScalarTruncation; use vortex::scalar::lower_bound; @@ -330,12 +330,12 @@ impl LayoutReader for CudaFlatReader { let array = array.apply(&expr)?; let array = array.filter(mask.clone())?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?.into_mask(); + let array_mask = array.null_as_false().execute::(&mut ctx)?; mask.intersect_by_rank(&array_mask) } else { let array = array.apply(&expr)?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?.into_mask(); + let array_mask = array.null_as_false().execute::(&mut ctx)?; mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/dict/reader.rs b/vortex-layout/src/layouts/dict/reader.rs index 19e002b42ee..3ed2153d387 100644 --- a/vortex-layout/src/layouts/dict/reader.rs +++ b/vortex-layout/src/layouts/dict/reader.rs @@ -21,7 +21,7 @@ use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; use vortex_array::expr::root; -use vortex_array::mask::MaskNullAsFalse; +use vortex_array::Executor; use vortex_array::optimizer::ArrayOptimizer; use vortex_error::VortexError; use vortex_error::VortexExpect; @@ -215,8 +215,7 @@ impl LayoutReader for DictReader { let mut ctx = session.create_execution_ctx(); let dict_mask = values .take(codes)? - .execute::(&mut ctx)? - .into_mask(); + .null_as_false().execute::(&mut ctx)?; Ok(mask.bitand(&dict_mask)) })) diff --git a/vortex-layout/src/layouts/flat/reader.rs b/vortex-layout/src/layouts/flat/reader.rs index 6b4e7b0c240..fdad53d7ff6 100644 --- a/vortex-layout/src/layouts/flat/reader.rs +++ b/vortex-layout/src/layouts/flat/reader.rs @@ -15,7 +15,7 @@ use vortex_array::VortexSessionExecute; use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; -use vortex_array::mask::MaskNullAsFalse; +use vortex_array::Executor; use vortex_array::serde::SerializedArray; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -156,14 +156,14 @@ impl LayoutReader for FlatReader { let array = array.apply(&expr)?; let array = array.filter(mask.clone())?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?.into_mask(); + let array_mask = array.null_as_false().execute::(&mut ctx)?; mask.intersect_by_rank(&array_mask) } else { // Run over the full array, with a simpler bitand at the end. let array = array.apply(&expr)?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.execute::(&mut ctx)?.into_mask(); + let array_mask = array.null_as_false().execute::(&mut ctx)?; mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/partitioned.rs b/vortex-layout/src/layouts/partitioned.rs index deaccd92513..94ccbc93d28 100644 --- a/vortex-layout/src/layouts/partitioned.rs +++ b/vortex-layout/src/layouts/partitioned.rs @@ -15,8 +15,9 @@ use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::expr::Expression; use vortex_array::expr::transform::PartitionedExpr; -use vortex_array::mask::MaskNullAsFalse; +use vortex_array::Executor; use vortex_array::validity::Validity; +use vortex_mask::Mask; use vortex_error::VortexError; use vortex_error::VortexResult; use vortex_session::VortexSession; @@ -92,8 +93,7 @@ impl PartitionedExprEval

for PartitionedExpr

{ let mut ctx = session.create_execution_ctx(); let root_mask = root_scope .apply(&self.root)? - .execute::(&mut ctx)? - .into_mask(); + .null_as_false().execute::(&mut ctx)?; let mask = mask.bitand(&root_mask); diff --git a/vortex-layout/src/layouts/row_idx/mod.rs b/vortex-layout/src/layouts/row_idx/mod.rs index 5693ae7dc38..468a5d9ef0f 100644 --- a/vortex-layout/src/layouts/row_idx/mod.rs +++ b/vortex-layout/src/layouts/row_idx/mod.rs @@ -32,7 +32,7 @@ use vortex_array::expr::root; use vortex_array::expr::transform::PartitionedExpr; use vortex_array::expr::transform::partition; use vortex_array::expr::transform::replace; -use vortex_array::mask::MaskNullAsFalse; +use vortex_array::Executor; use vortex_array::scalar::PValue; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -298,8 +298,7 @@ fn row_idx_mask_future( let mut ctx = session.create_execution_ctx(); let result_mask = array .apply(&expr)? - .execute::(&mut ctx)? - .into_mask(); + .null_as_false().execute::(&mut ctx)?; Ok(result_mask.bitand(&mask.await?)) }) diff --git a/vortex-layout/src/layouts/zoned/zone_map.rs b/vortex-layout/src/layouts/zoned/zone_map.rs index 69b98d5e41a..a3827578055 100644 --- a/vortex-layout/src/layouts/zoned/zone_map.rs +++ b/vortex-layout/src/layouts/zoned/zone_map.rs @@ -14,7 +14,7 @@ use vortex_array::arrays::StructArray; use vortex_array::dtype::DType; use vortex_array::expr::Expression; use vortex_array::expr::stats::Stat; -use vortex_array::mask::MaskNullAsFalse; +use vortex_array::Executor; use vortex_array::scalar_fn::internal::row_count::contains_row_count; use vortex_array::scalar_fn::internal::row_count::substitute_row_count; use vortex_array::validity::Validity; @@ -100,14 +100,13 @@ impl ZoneMap { let applied = self.array.clone().into_array().apply(predicate)?; if num_zones == 0 || !contains_row_count(&applied) { - return Ok(applied.execute::(&mut ctx)?.into_mask()); + return Ok(applied.null_as_false().execute::(&mut ctx)?); } let row_count_array = row_count_array(self.zone_len, self.row_count, num_zones)?; let substituted = substitute_row_count(applied, &row_count_array)?; Ok(substituted - .execute::(&mut ctx)? - .into_mask()) + .null_as_false().execute::(&mut ctx)?) } } diff --git a/vortex/src/lib.rs b/vortex/src/lib.rs index b5241797a01..94a6b994df5 100644 --- a/vortex/src/lib.rs +++ b/vortex/src/lib.rs @@ -76,7 +76,7 @@ pub mod layout { } pub mod mask { - pub use vortex_array::mask::MaskNullAsFalse; + pub use vortex_array::mask::NullAsFalse; pub use vortex_mask::*; } From e8b77a5599c65f48640cba9c292377ee6648e8c9 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Mon, 15 Jun 2026 13:36:51 +0000 Subject: [PATCH 5/6] Replace NullAsFalse adapter with array.fill_null(false) Remove the NullAsFalse executor adapter and its supporting buffer helpers. Mask::execute keeps rejecting nullable boolean arrays; callers that need null-as-false semantics now call array.fill_null(false) before executing into a Mask. Signed-off-by: Joe Isaacs --- .../src/aggregate_fn/fns/is_constant/mod.rs | 3 +- vortex-array/src/executor.rs | 32 ++---- vortex-array/src/mask.rs | 106 +++--------------- vortex-buffer/src/bit/buf.rs | 54 --------- vortex-buffer/src/bit/ops.rs | 42 ------- vortex-cuda/src/layout.rs | 6 +- vortex-layout/src/layouts/dict/reader.rs | 5 +- vortex-layout/src/layouts/flat/reader.rs | 6 +- vortex-layout/src/layouts/partitioned.rs | 7 +- vortex-layout/src/layouts/row_idx/mod.rs | 5 +- vortex-layout/src/layouts/zoned/zone_map.rs | 7 +- vortex/src/lib.rs | 1 - 12 files changed, 40 insertions(+), 234 deletions(-) diff --git a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs index 18d4165fcc5..d1dd6bef39c 100644 --- a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs @@ -27,7 +27,6 @@ use crate::ArrayRef; use crate::Canonical; use crate::Columnar; use crate::ExecutionCtx; -use crate::Executor; use crate::IntoArray; use crate::aggregate_fn::Accumulator; use crate::aggregate_fn::AggregateFnId; @@ -75,7 +74,7 @@ fn arrays_value_equal(a: &ArrayRef, b: &ArrayRef, ctx: &mut ExecutionCtx) -> Vor // Compare values element-wise. Result is null where both inputs are null, // true/false where both are valid. let eq_result = a.binary(b.clone(), Operator::Eq)?; - let eq_result = eq_result.null_as_false().execute::(ctx)?; + let eq_result = eq_result.fill_null(false)?.execute::(ctx)?; Ok(eq_result.true_count() == valid_count) } diff --git a/vortex-array/src/executor.rs b/vortex-array/src/executor.rs index a102456e765..d6070ac1a4d 100644 --- a/vortex-array/src/executor.rs +++ b/vortex-array/src/executor.rs @@ -62,33 +62,15 @@ pub(crate) fn max_iterations() -> usize { *MAX_ITERATIONS } -/// Trait for types that a `Source` can be executed into. +/// Marker trait for types that an [`ArrayRef`] can be executed into. /// -/// `Source` defaults to [`ArrayRef`], so `impl Executable for X` defines how to execute an -/// array into an `X`. Other sources are lightweight stack-allocated [`Executor`] adapters -/// that adjust execution semantics without wrapping the array in another array node, e.g. -/// `impl Executable for Mask` executes a nullable boolean array into a -/// [`vortex_mask::Mask`], coercing nulls to `false`. +/// Implementors must provide an implementation of `execute` that takes +/// an [`ArrayRef`] and an [`ExecutionCtx`], and produces an instance of the +/// implementor type. /// -/// Users should use the [`Executor::execute`] method (or the inherent `ArrayRef::execute` -/// and `ArrayRef::execute_as` methods) rather than calling this trait directly. -pub trait Executable: Sized { - fn execute(source: Source, ctx: &mut ExecutionCtx) -> VortexResult; -} - -/// Types that can drive execution into an [`Executable`] target. -/// -/// Implemented by [`ArrayRef`] and by stack-allocated adapters such as -/// [`NullAsFalse`](crate::mask::NullAsFalse): -/// -/// ```ignore -/// let mask = array.null_as_false().execute::(ctx)?; -/// ``` -pub trait Executor: Sized { - /// Execute into an instance of `E`. - fn execute>(self, ctx: &mut ExecutionCtx) -> VortexResult { - E::execute(self, ctx) - } +/// Users should use the `Array::execute` or `Array::execute_as` methods +pub trait Executable: Sized { + fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult; } #[expect(clippy::same_name_method)] diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 13f79161d93..9c8508f980c 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -8,20 +8,18 @@ use vortex_mask::Mask; use crate::ArrayRef; use crate::Executable; use crate::ExecutionCtx; -use crate::Executor; use crate::IntoArray; use crate::arrays::BoolArray; use crate::columnar::Columnar; use crate::dtype::DType; use crate::dtype::Nullability; -use crate::validity::Validity; impl Executable for Mask { /// Executes a boolean array into a [`Mask`]. /// - /// The array must have a non-nullable boolean dtype. Use - /// [`ArrayRef::null_as_false`] to execute a nullable boolean array, coercing null - /// elements to `false`. + /// The array must have a non-nullable boolean dtype. To execute a nullable boolean array, + /// coercing null elements to `false`, first call + /// [`ArrayRef::fill_null(false)`](crate::builtins::ArrayBuiltins::fill_null). fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { if !matches!(array.dtype(), DType::Bool(Nullability::NonNullable)) { vortex_bail!( @@ -43,119 +41,41 @@ impl Executable for Mask { } } -/// An [`Executor`] adapter that coerces null elements of a boolean array to `false` before -/// executing into the target type. -/// -/// Created by [`ArrayRef::null_as_false`]. The adapter lives on the stack and moves the -/// underlying [`ArrayRef`], so it does not allocate a wrapper array node. -/// -/// Use for filter and pruning predicates over nullable data, where SQL semantics treat -/// `NULL` as not matching: -/// -/// ```ignore -/// let mask = array.null_as_false().execute::(ctx)?; -/// ``` -pub struct NullAsFalse(ArrayRef); - -impl ArrayRef { - /// Returns an [`Executor`] that treats null elements of this boolean array as `false`. - pub fn null_as_false(self) -> NullAsFalse { - NullAsFalse(self) - } -} - -impl Executor for NullAsFalse {} - -impl Executable for Mask { - /// Executes a boolean array into a [`Mask`], coercing null elements to `false`. - /// - /// The validity is folded into the value bits with a single AND that reuses the value - /// buffer when it is uniquely owned. - fn execute(source: NullAsFalse, ctx: &mut ExecutionCtx) -> VortexResult { - let array = source.0; - if !matches!(array.dtype(), DType::Bool(_)) { - vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); - } - if !array.dtype().is_nullable() { - return array.execute(ctx); - } - - let len = array.len(); - Ok(match array.execute::(ctx)? { - Columnar::Constant(c) => { - Mask::new(len, c.scalar().as_bool().value().unwrap_or(false)) - } - Columnar::Canonical(c) => { - let bool = c.into_array().execute::(ctx)?; - match bool.as_ref().validity()? { - Validity::NonNullable | Validity::AllValid => { - Mask::from_buffer(bool.into_bit_buffer()) - } - Validity::AllInvalid => Mask::new_false(len), - Validity::Array(v) => { - let validity_bits = v.execute::(ctx)?.into_bit_buffer(); - Mask::from_buffer(bool.into_bit_buffer().bitand_in_place(&validity_bits)) - } - } - } - }) - } -} - #[cfg(test)] mod tests { use vortex_error::VortexResult; use vortex_mask::Mask; use crate::ExecutionCtx; - use crate::Executor; use crate::IntoArray; use crate::LEGACY_SESSION; use crate::VortexSessionExecute; use crate::arrays::BoolArray; - use crate::arrays::ConstantArray; - use crate::dtype::DType; - use crate::dtype::Nullability; - use crate::scalar::Scalar; + use crate::builtins::ArrayBuiltins; fn ctx() -> ExecutionCtx { LEGACY_SESSION.create_execution_ctx() } #[test] - fn null_as_false_non_nullable() -> VortexResult<()> { + fn mask_non_nullable() -> VortexResult<()> { let array = BoolArray::from_iter([true, false, true]).into_array(); - let mask = array.null_as_false().execute::(&mut ctx())?; + let mask = array.execute::(&mut ctx())?; assert_eq!(mask, Mask::from_iter([true, false, true])); Ok(()) } #[test] - fn null_as_false_coerces_nulls() -> VortexResult<()> { - let array = BoolArray::from_iter([Some(true), None, Some(false), None]).into_array(); - let mask = array.null_as_false().execute::(&mut ctx())?; - assert_eq!(mask, Mask::from_iter([true, false, false, false])); - Ok(()) + fn mask_rejects_nullable() { + let array = BoolArray::from_iter([Some(true), None]).into_array(); + assert!(array.execute::(&mut ctx()).is_err()); } #[test] - fn null_as_false_null_constant() -> VortexResult<()> { - let array = - ConstantArray::new(Scalar::null(DType::Bool(Nullability::Nullable)), 4).into_array(); - let mask = array.null_as_false().execute::(&mut ctx())?; - assert_eq!(mask, Mask::new_false(4)); + fn fill_null_then_mask_coerces_nulls() -> VortexResult<()> { + let array = BoolArray::from_iter([Some(true), None, Some(false), None]).into_array(); + let mask = array.fill_null(false)?.execute::(&mut ctx())?; + assert_eq!(mask, Mask::from_iter([true, false, false, false])); Ok(()) } - - #[test] - fn null_as_false_rejects_non_bool() { - let array = ConstantArray::new(42i32, 4).into_array(); - assert!(array.null_as_false().execute::(&mut ctx()).is_err()); - } - - #[test] - fn mask_rejects_nullable() { - let array = BoolArray::from_iter([Some(true), None]).into_array(); - assert!(array.execute::(&mut ctx()).is_err()); - } } diff --git a/vortex-buffer/src/bit/buf.rs b/vortex-buffer/src/bit/buf.rs index a9dac5bac09..457827f4ab9 100644 --- a/vortex-buffer/src/bit/buf.rs +++ b/vortex-buffer/src/bit/buf.rs @@ -25,7 +25,6 @@ use crate::bit::collect_bool_word; use crate::bit::count_ones::count_ones; use crate::bit::get_bit_unchecked; use crate::bit::ops::bitwise_binary_op; -use crate::bit::ops::bitwise_binary_op_mut; use crate::bit::ops::bitwise_binary_op_lhs_owned; use crate::bit::ops::bitwise_unary_op; use crate::bit::ops::bitwise_unary_op_copy; @@ -542,27 +541,6 @@ impl BitBuffer { bitwise_binary_op_lhs_owned(self, rhs, |a, b| a & !b) } - /// Compute `self & rhs`, reusing `self`'s allocation when it is uniquely owned and - /// byte-aligned. - /// - /// Falls back to an allocating AND when `self` is shared or its offset is not a multiple - /// of 8. `rhs` may have any offset. - pub fn bitand_in_place(self, rhs: &BitBuffer) -> BitBuffer { - assert_eq!(self.len(), rhs.len(), "Buffers must have the same length"); - - if !self.offset().is_multiple_of(8) { - return &self & rhs; - } - - match self.try_into_mut() { - Ok(mut lhs) => { - bitwise_binary_op_mut(&mut lhs, rhs, |a, b| a & b); - lhs.freeze() - } - Err(this) => &this & rhs, - } - } - /// Iterate through bits in a buffer. /// /// # Arguments @@ -621,38 +599,6 @@ mod tests { use crate::bit::BitBuffer; use crate::buffer; - #[rstest] - #[case(0, 0)] // aligned lhs, aligned rhs - #[case(0, 3)] // aligned lhs, misaligned rhs - #[case(5, 0)] // misaligned lhs (falls back to allocating path) - #[case(11, 7)] // both misaligned - fn test_bitand_in_place(#[case] lhs_offset: usize, #[case] rhs_offset: usize) { - let len = 200; - let lhs = BitBuffer::from_iter((0..len + lhs_offset).map(|i| i % 2 == 0)) - .slice(lhs_offset..len + lhs_offset); - let rhs = BitBuffer::from_iter((0..len + rhs_offset).map(|i| i % 3 == 0)) - .slice(rhs_offset..len + rhs_offset); - - let expected = &lhs & &rhs; - let actual = lhs.bitand_in_place(&rhs); - - assert_eq!(actual.len(), expected.len()); - assert!(actual.iter().eq(expected.iter())); - } - - #[test] - fn test_bitand_in_place_shared_falls_back() { - let lhs = BitBuffer::from_iter((0..100).map(|i| i % 2 == 0)); - let shared = lhs.clone(); - let rhs = BitBuffer::from_iter((0..100).map(|i| i % 5 == 0)); - - let actual = lhs.bitand_in_place(&rhs); - let expected = &shared & &rhs; - assert!(actual.iter().eq(expected.iter())); - // The shared copy must be unmodified. - assert!(shared.iter().eq((0..100).map(|i| i % 2 == 0))); - } - #[test] fn test_bool() { // Create a new Buffer of length 1024 where the 8th bit is set. diff --git a/vortex-buffer/src/bit/ops.rs b/vortex-buffer/src/bit/ops.rs index d39618fff29..be1401d912a 100644 --- a/vortex-buffer/src/bit/ops.rs +++ b/vortex-buffer/src/bit/ops.rs @@ -182,48 +182,6 @@ pub(super) fn bitwise_binary_op_lhs_owned u64>( } } -/// Applies `op` word-wise to `lhs` in place, zipping with the logical words of `rhs`. -/// -/// `lhs` must be byte-aligned (`offset % 8 == 0`) and have the same length as `rhs`. Bits -/// beyond `lhs.len()` in the final byte are left unspecified after this call. -pub(super) fn bitwise_binary_op_mut u64>( - lhs: &mut BitBufferMut, - rhs: &BitBuffer, - mut op: F, -) { - assert_eq!(lhs.len(), rhs.len(), "Buffers must have the same length"); - assert!( - lhs.offset().is_multiple_of(8), - "lhs must be byte-aligned for in-place bitwise ops" - ); - - let byte_start = lhs.offset() / 8; - let byte_len = lhs.len().div_ceil(8); - let slice = &mut lhs.as_mut_slice()[byte_start..byte_start + byte_len]; - - // `iter_padded` yields exactly `len.div_ceil(64)` logical words, which matches the number - // of words consumed below: `byte_len / 8` full words plus one for any remainder bytes. - let mut rhs_words = rhs.chunks().iter_padded(); - - let mut chunks = slice.chunks_exact_mut(8); - for chunk in &mut chunks { - let rhs_word = rhs_words.next().unwrap_or(0); - let mut bytes = [0u8; 8]; - bytes.copy_from_slice(chunk); - let value = op(u64::from_le_bytes(bytes), rhs_word); - chunk.copy_from_slice(&value.to_le_bytes()); - } - - let remainder = chunks.into_remainder(); - if !remainder.is_empty() { - let rhs_word = rhs_words.next().unwrap_or(0); - let mut bytes = [0u8; 8]; - bytes[..remainder.len()].copy_from_slice(remainder); - let value = op(u64::from_le_bytes(bytes), rhs_word); - remainder.copy_from_slice(&value.to_le_bytes()[..remainder.len()]); - } -} - pub(super) fn bitwise_binary_op u64>( left: &BitBuffer, right: &BitBuffer, diff --git a/vortex-cuda/src/layout.rs b/vortex-cuda/src/layout.rs index f097805921c..9e6111fc371 100644 --- a/vortex-cuda/src/layout.rs +++ b/vortex-cuda/src/layout.rs @@ -22,6 +22,7 @@ use vortex::array::MaskFuture; use vortex::array::ProstMetadata; use vortex::array::VortexSessionExecute; use vortex::array::arrays::Constant; +use vortex::array::builtins::ArrayBuiltins; use vortex::array::expr::Expression; use vortex::array::expr::stats::Precision; use vortex::array::expr::stats::Stat; @@ -58,7 +59,6 @@ use vortex::layout::segments::SegmentSource; use vortex::layout::sequence::SendableSequentialStream; use vortex::layout::sequence::SequencePointer; use vortex::layout::vtable; -use vortex::array::Executor; use vortex::mask::Mask; use vortex::scalar::Scalar; use vortex::scalar::ScalarTruncation; @@ -332,12 +332,12 @@ impl LayoutReader for CudaFlatReader { let array = array.apply(&expr)?; let array = array.filter(mask.clone())?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.null_as_false().execute::(&mut ctx)?; + let array_mask = array.fill_null(false)?.execute::(&mut ctx)?; mask.intersect_by_rank(&array_mask) } else { let array = array.apply(&expr)?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.null_as_false().execute::(&mut ctx)?; + let array_mask = array.fill_null(false)?.execute::(&mut ctx)?; mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/dict/reader.rs b/vortex-layout/src/layouts/dict/reader.rs index cc2eaf7cbcd..6c8d91706db 100644 --- a/vortex-layout/src/layouts/dict/reader.rs +++ b/vortex-layout/src/layouts/dict/reader.rs @@ -16,11 +16,11 @@ use vortex_array::MaskFuture; use vortex_array::VortexSessionExecute; use vortex_array::arrays::DictArray; use vortex_array::arrays::SharedArray; +use vortex_array::builtins::ArrayBuiltins; use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; use vortex_array::expr::root; -use vortex_array::Executor; use vortex_array::optimizer::ArrayOptimizer; use vortex_error::VortexError; use vortex_error::VortexExpect; @@ -219,7 +219,8 @@ impl LayoutReader for DictReader { let mut ctx = session.create_execution_ctx(); let dict_mask = values .take(codes)? - .null_as_false().execute::(&mut ctx)?; + .fill_null(false)? + .execute::(&mut ctx)?; Ok(mask.bitand(&dict_mask)) })) diff --git a/vortex-layout/src/layouts/flat/reader.rs b/vortex-layout/src/layouts/flat/reader.rs index c72b6d56a00..e03ffc20e1a 100644 --- a/vortex-layout/src/layouts/flat/reader.rs +++ b/vortex-layout/src/layouts/flat/reader.rs @@ -11,10 +11,10 @@ use tracing::trace; use vortex_array::ArrayRef; use vortex_array::MaskFuture; use vortex_array::VortexSessionExecute; +use vortex_array::builtins::ArrayBuiltins; use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; -use vortex_array::Executor; use vortex_array::serde::SerializedArray; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -157,14 +157,14 @@ impl LayoutReader for FlatReader { let array = array.apply(&expr)?; let array = array.filter(mask.clone())?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.null_as_false().execute::(&mut ctx)?; + let array_mask = array.fill_null(false)?.execute::(&mut ctx)?; mask.intersect_by_rank(&array_mask) } else { // Run over the full array, with a simpler bitand at the end. let array = array.apply(&expr)?; let mut ctx = session.create_execution_ctx(); - let array_mask = array.null_as_false().execute::(&mut ctx)?; + let array_mask = array.fill_null(false)?.execute::(&mut ctx)?; mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/partitioned.rs b/vortex-layout/src/layouts/partitioned.rs index 94ccbc93d28..cd927bbc074 100644 --- a/vortex-layout/src/layouts/partitioned.rs +++ b/vortex-layout/src/layouts/partitioned.rs @@ -11,15 +11,15 @@ use vortex_array::IntoArray; use vortex_array::MaskFuture; use vortex_array::VortexSessionExecute; use vortex_array::arrays::StructArray; +use vortex_array::builtins::ArrayBuiltins; use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::expr::Expression; use vortex_array::expr::transform::PartitionedExpr; -use vortex_array::Executor; use vortex_array::validity::Validity; -use vortex_mask::Mask; use vortex_error::VortexError; use vortex_error::VortexResult; +use vortex_mask::Mask; use vortex_session::VortexSession; use crate::ArrayFuture; @@ -93,7 +93,8 @@ impl PartitionedExprEval

for PartitionedExpr

{ let mut ctx = session.create_execution_ctx(); let root_mask = root_scope .apply(&self.root)? - .null_as_false().execute::(&mut ctx)?; + .fill_null(false)? + .execute::(&mut ctx)?; let mask = mask.bitand(&root_mask); diff --git a/vortex-layout/src/layouts/row_idx/mod.rs b/vortex-layout/src/layouts/row_idx/mod.rs index 783405fa258..2347d05b0bd 100644 --- a/vortex-layout/src/layouts/row_idx/mod.rs +++ b/vortex-layout/src/layouts/row_idx/mod.rs @@ -19,6 +19,7 @@ use vortex_array::Canonical; use vortex_array::IntoArray; use vortex_array::MaskFuture; use vortex_array::VortexSessionExecute; +use vortex_array::builtins::ArrayBuiltins; use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::dtype::FieldName; @@ -31,7 +32,6 @@ use vortex_array::expr::root; use vortex_array::expr::transform::PartitionedExpr; use vortex_array::expr::transform::partition; use vortex_array::expr::transform::replace; -use vortex_array::Executor; use vortex_array::scalar::PValue; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -298,7 +298,8 @@ fn row_idx_mask_future( let mut ctx = session.create_execution_ctx(); let result_mask = array .apply(&expr)? - .null_as_false().execute::(&mut ctx)?; + .fill_null(false)? + .execute::(&mut ctx)?; Ok(result_mask.bitand(&mask.await?)) }) diff --git a/vortex-layout/src/layouts/zoned/zone_map.rs b/vortex-layout/src/layouts/zoned/zone_map.rs index 7d6b21e74de..d7a5b7cd769 100644 --- a/vortex-layout/src/layouts/zoned/zone_map.rs +++ b/vortex-layout/src/layouts/zoned/zone_map.rs @@ -17,6 +17,7 @@ use vortex_array::arrays::ConstantArray; use vortex_array::arrays::PrimitiveArray; use vortex_array::arrays::StructArray; use vortex_array::arrays::struct_::StructArrayExt; +use vortex_array::builtins::ArrayBuiltins; use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::expr::Expression; @@ -33,7 +34,6 @@ use vortex_array::scalar_fn::EmptyOptions; use vortex_array::scalar_fn::ScalarFnVTableExt; use vortex_array::scalar_fn::fns::stat::StatFn; use vortex_array::scalar_fn::internal::row_count::RowCount; -use vortex_array::Executor; use vortex_array::scalar_fn::internal::row_count::contains_row_count; use vortex_array::scalar_fn::internal::row_count::substitute_row_count; use vortex_array::validity::Validity; @@ -124,13 +124,12 @@ impl ZoneMap { let applied = self.array.clone().into_array().apply(&predicate)?; if !contains_row_count(&applied) { - return applied.null_as_false().execute::(&mut ctx)?; + return applied.fill_null(false)?.execute::(&mut ctx); } let row_count_array = row_count_array(self.zone_len, self.row_count, num_zones)?; let substituted = substitute_row_count(applied, &row_count_array)?; - Ok(substituted - .null_as_false().execute::(&mut ctx)?) + substituted.fill_null(false)?.execute::(&mut ctx) } fn lower_stats(&self, predicate: Expression) -> VortexResult { diff --git a/vortex/src/lib.rs b/vortex/src/lib.rs index 3903b6fd439..baf0d0ae761 100644 --- a/vortex/src/lib.rs +++ b/vortex/src/lib.rs @@ -77,7 +77,6 @@ pub mod layout { } pub mod mask { - pub use vortex_array::mask::NullAsFalse; pub use vortex_mask::*; } From 81de01ea86b46fb497a05ce9ec8fb4d844bee91a Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Mon, 15 Jun 2026 20:41:01 +0000 Subject: [PATCH 6/6] test: cover all-null constant in fill_null then mask Signed-off-by: Joe Isaacs --- vortex-array/src/mask.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 9c8508f980c..09032aabbd0 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -51,7 +51,11 @@ mod tests { use crate::LEGACY_SESSION; use crate::VortexSessionExecute; use crate::arrays::BoolArray; + use crate::arrays::ConstantArray; use crate::builtins::ArrayBuiltins; + use crate::dtype::DType; + use crate::dtype::Nullability; + use crate::scalar::Scalar; fn ctx() -> ExecutionCtx { LEGACY_SESSION.create_execution_ctx() @@ -78,4 +82,13 @@ mod tests { assert_eq!(mask, Mask::from_iter([true, false, false, false])); Ok(()) } + + #[test] + fn fill_null_then_mask_null_constant() -> VortexResult<()> { + let array = + ConstantArray::new(Scalar::null(DType::Bool(Nullability::Nullable)), 4).into_array(); + let mask = array.fill_null(false)?.execute::(&mut ctx())?; + assert_eq!(mask, Mask::new_false(4)); + Ok(()) + } }