From 116bac51b07b4c2f82d0e85baa4ed9bfd1d6e966 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Wed, 17 Jun 2026 14:06:21 +0100 Subject: [PATCH 1/4] perf: cheap null-coercion for filter masks (fix ClickBench Q22 regression) #8121 inserted `array.fill_null(false)?` before every `execute::` filter call site. `fill_null` is a no-op only for non-nullable arrays; for nullable predicate results (e.g. `LIKE`/`<>` over nullable columns, as ClickBench string columns are stored) it takes the expensive path: `precondition` calls `array.validity()?` on the lazy predicate array, re-deriving validity from the expression tree and materializing an intermediate array, before `execute::` canonicalizes again. This regressed ClickBench Q22. Add `execute_mask_coercing_nulls`, which canonicalizes the array once and folds validity into the value bits with a single bitmap AND (the pre-#8121 cost profile), and route the filter readers (dict/flat/partitioned/row_idx/zone_map, CUDA, is_constant) through it. `Mask::execute` stays strict about nullable input. Signed-off-by: Joe Isaacs Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/aggregate_fn/fns/is_constant/mod.rs | 4 +- vortex-array/src/mask.rs | 72 +++++++++++++++++++ vortex-cuda/src/layout.rs | 6 +- vortex-layout/src/layouts/dict/reader.rs | 7 +- vortex-layout/src/layouts/flat/reader.rs | 6 +- vortex-layout/src/layouts/partitioned.rs | 8 +-- vortex-layout/src/layouts/row_idx/mod.rs | 7 +- vortex-layout/src/layouts/zoned/zone_map.rs | 6 +- 8 files changed, 89 insertions(+), 27 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 862437ebcdc..3d6317666d4 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::execute_mask_coercing_nulls; use crate::scalar::Scalar; use crate::scalar_fn::fns::operators::Operator; @@ -73,7 +73,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.fill_null(false)?.execute::(ctx)?; + let eq_result = execute_mask_coercing_nulls(eq_result, ctx)?; Ok(eq_result.true_count() == valid_count) } diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 09032aabbd0..99d43e4a7e3 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -1,6 +1,8 @@ // 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; @@ -41,6 +43,37 @@ impl Executable for Mask { } } +/// Executes a boolean array into a [`Mask`], coercing null elements to `false` (SQL-style +/// `NULL`-as-not-matching semantics). +/// +/// This is the cheap counterpart to [`ArrayRef::fill_null(false)`](crate::builtins::ArrayBuiltins::fill_null) +/// followed by [`Mask::execute`]. It canonicalizes the (possibly lazy) array exactly once and folds +/// validity into the value bits with a single bitmap `AND`, rather than routing through the generic +/// `fill_null` scalar function, which derives validity from the lazy expression tree (re-evaluating +/// predicates such as `LIKE`) and materializes an intermediate array. +pub fn execute_mask_coercing_nulls(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { + 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)) + } + Columnar::Canonical(a) => { + let bool = a.into_array().execute::(ctx)?; + // Treat nulls as `false`: fold validity into the value bits. + let validity = bool + .as_ref() + .validity()? + .execute_mask(bool.as_ref().len(), ctx)?; + let bits = bool.into_bit_buffer(); + validity.bitand(&Mask::from(bits)) + } + }) +} + #[cfg(test)] mod tests { use vortex_error::VortexResult; @@ -55,6 +88,7 @@ mod tests { use crate::builtins::ArrayBuiltins; use crate::dtype::DType; use crate::dtype::Nullability; + use crate::mask::execute_mask_coercing_nulls; use crate::scalar::Scalar; fn ctx() -> ExecutionCtx { @@ -91,4 +125,42 @@ mod tests { assert_eq!(mask, Mask::new_false(4)); Ok(()) } + + #[test] + fn coercing_nulls_non_nullable() -> VortexResult<()> { + let array = BoolArray::from_iter([true, false, true]).into_array(); + let mask = execute_mask_coercing_nulls(array, &mut ctx())?; + assert_eq!(mask, Mask::from_iter([true, false, true])); + Ok(()) + } + + #[test] + fn coercing_nulls_treats_null_as_false() -> VortexResult<()> { + let array = BoolArray::from_iter([Some(true), None, Some(false), None]).into_array(); + let mask = execute_mask_coercing_nulls(array, &mut ctx())?; + assert_eq!(mask, Mask::from_iter([true, false, false, false])); + Ok(()) + } + + #[test] + fn coercing_nulls_null_constant() -> VortexResult<()> { + let array = + ConstantArray::new(Scalar::null(DType::Bool(Nullability::Nullable)), 4).into_array(); + let mask = execute_mask_coercing_nulls(array, &mut ctx())?; + assert_eq!(mask, Mask::new_false(4)); + Ok(()) + } + + #[test] + fn coercing_nulls_matches_fill_null_then_mask() -> VortexResult<()> { + let array = + BoolArray::from_iter([Some(true), None, Some(false), Some(true), None]).into_array(); + let via_fill_null = array + .clone() + .fill_null(false)? + .execute::(&mut ctx())?; + let via_coerce = execute_mask_coercing_nulls(array, &mut ctx())?; + assert_eq!(via_coerce, via_fill_null); + Ok(()) + } } diff --git a/vortex-cuda/src/layout.rs b/vortex-cuda/src/layout.rs index 9e6111fc371..c6ddaece84b 100644 --- a/vortex-cuda/src/layout.rs +++ b/vortex-cuda/src/layout.rs @@ -22,11 +22,11 @@ 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; use vortex::array::expr::stats::StatsProvider; +use vortex::array::mask::execute_mask_coercing_nulls; use vortex::array::normalize::NormalizeOptions; use vortex::array::normalize::Operation; use vortex::array::serde::SerializeOptions; @@ -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.fill_null(false)?.execute::(&mut ctx)?; + let array_mask = execute_mask_coercing_nulls(array, &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.fill_null(false)?.execute::(&mut ctx)?; + let array_mask = execute_mask_coercing_nulls(array, &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 26749e42f46..3219afce46c 100644 --- a/vortex-layout/src/layouts/dict/reader.rs +++ b/vortex-layout/src/layouts/dict/reader.rs @@ -16,7 +16,6 @@ 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::dtype::Nullability; @@ -27,6 +26,7 @@ use vortex_array::expr::label_tree; use vortex_array::expr::pack; use vortex_array::expr::root; use vortex_array::expr::transform::partition_annotations; +use vortex_array::mask::execute_mask_coercing_nulls; use vortex_array::optimizer::ArrayOptimizer; use vortex_array::scalar_fn::is_negative_cost; use vortex_error::VortexError; @@ -259,10 +259,7 @@ impl LayoutReader for DictReader { let mask = mask.await?; let mut ctx = session.create_execution_ctx(); - let dict_mask = values - .take(codes)? - .fill_null(false)? - .execute::(&mut ctx)?; + let dict_mask = execute_mask_coercing_nulls(values.take(codes)?, &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 e03ffc20e1a..d2b53f56d4d 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::mask::execute_mask_coercing_nulls; 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.fill_null(false)?.execute::(&mut ctx)?; + let array_mask = execute_mask_coercing_nulls(array, &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.fill_null(false)?.execute::(&mut ctx)?; + let array_mask = execute_mask_coercing_nulls(array, &mut ctx)?; mask.bitand(&array_mask) }; diff --git a/vortex-layout/src/layouts/partitioned.rs b/vortex-layout/src/layouts/partitioned.rs index cd927bbc074..752d90d4532 100644 --- a/vortex-layout/src/layouts/partitioned.rs +++ b/vortex-layout/src/layouts/partitioned.rs @@ -11,15 +11,14 @@ 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::mask::execute_mask_coercing_nulls; use vortex_array::validity::Validity; use vortex_error::VortexError; use vortex_error::VortexResult; -use vortex_mask::Mask; use vortex_session::VortexSession; use crate::ArrayFuture; @@ -91,10 +90,7 @@ impl PartitionedExprEval

for PartitionedExpr

{ .into_array(); let mut ctx = session.create_execution_ctx(); - let root_mask = root_scope - .apply(&self.root)? - .fill_null(false)? - .execute::(&mut ctx)?; + let root_mask = execute_mask_coercing_nulls(root_scope.apply(&self.root)?, &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 2347d05b0bd..f28f1180592 100644 --- a/vortex-layout/src/layouts/row_idx/mod.rs +++ b/vortex-layout/src/layouts/row_idx/mod.rs @@ -19,7 +19,6 @@ 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; @@ -32,6 +31,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::execute_mask_coercing_nulls; use vortex_array::scalar::PValue; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -296,10 +296,7 @@ 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)? - .fill_null(false)? - .execute::(&mut ctx)?; + let result_mask = execute_mask_coercing_nulls(array.apply(&expr)?, &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 d7a5b7cd769..85acadbc7c1 100644 --- a/vortex-layout/src/layouts/zoned/zone_map.rs +++ b/vortex-layout/src/layouts/zoned/zone_map.rs @@ -17,7 +17,6 @@ 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; @@ -29,6 +28,7 @@ use vortex_array::expr::root; use vortex_array::expr::stats::Stat; use vortex_array::expr::traversal::NodeExt; use vortex_array::expr::traversal::Transformed; +use vortex_array::mask::execute_mask_coercing_nulls; use vortex_array::scalar::Scalar; use vortex_array::scalar_fn::EmptyOptions; use vortex_array::scalar_fn::ScalarFnVTableExt; @@ -124,12 +124,12 @@ impl ZoneMap { let applied = self.array.clone().into_array().apply(&predicate)?; if !contains_row_count(&applied) { - return applied.fill_null(false)?.execute::(&mut ctx); + return execute_mask_coercing_nulls(applied, &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)?; - substituted.fill_null(false)?.execute::(&mut ctx) + execute_mask_coercing_nulls(substituted, &mut ctx) } fn lower_stats(&self, predicate: Expression) -> VortexResult { From 450e449c43bd6a2c96d307ca9687a158a1a89b62 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Wed, 17 Jun 2026 14:52:23 +0100 Subject: [PATCH 2/4] Refactor coercion into ArrayRef::null_as_false adapter Replace the execute_mask_coercing_nulls helper with an Executor-style `ArrayRef::null_as_false()` adapter (matching the design in #8121's history): canonicalize once and fold validity into the value bits with a single AND that reuses the value buffer when uniquely owned, deferring non-nullable input to the strict `Mask::execute`. Add the mask_fill_null microbenchmark and a one-line note that `fill_null` on a lazy `ScalarFn` array is currently slow. Verified on CI ClickBench: clickbench_q22/duckdb:vortex 1126ms -> 580ms (0.51, ~2x). Signed-off-by: Joe Isaacs Co-Authored-By: Claude Opus 4.8 (1M context) --- vortex-array/Cargo.toml | 4 + vortex-array/benches/mask_fill_null.rs | 172 ++++++++++++++++++ .../src/aggregate_fn/fns/is_constant/mod.rs | 3 +- vortex-array/src/mask.rs | 91 +++++---- vortex-cuda/src/layout.rs | 5 +- vortex-layout/src/layouts/dict/reader.rs | 3 +- vortex-layout/src/layouts/flat/reader.rs | 5 +- vortex-layout/src/layouts/partitioned.rs | 6 +- vortex-layout/src/layouts/row_idx/mod.rs | 3 +- vortex-layout/src/layouts/zoned/zone_map.rs | 5 +- 10 files changed, 245 insertions(+), 52 deletions(-) create mode 100644 vortex-array/benches/mask_fill_null.rs diff --git a/vortex-array/Cargo.toml b/vortex-array/Cargo.toml index de3bb416985..1886c92268f 100644 --- a/vortex-array/Cargo.toml +++ b/vortex-array/Cargo.toml @@ -101,6 +101,10 @@ vortex-array = { path = ".", features = ["_test-harness", "table-display"] } name = "aggregate_max" harness = false +[[bench]] +name = "mask_fill_null" +harness = false + [[bench]] name = "aggregate_sum" harness = false diff --git a/vortex-array/benches/mask_fill_null.rs b/vortex-array/benches/mask_fill_null.rs new file mode 100644 index 00000000000..443e04c3148 --- /dev/null +++ b/vortex-array/benches/mask_fill_null.rs @@ -0,0 +1,172 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +//! Microbenchmark isolating why `fill_null(false)` + `Mask::execute` is slow on a *lazy*, +//! nullable predicate result (the shape produced by the dict layout reader for a filter such as +//! `URL LIKE '%google%'` over a nullable, dictionary-encoded string column). +//! +//! The lazy array is `Dict{ codes, values: distinct.apply(like) }`. Executing it into a `Mask` +//! has two strategies: +//! +//! * `fill_null_then_mask` — `array.fill_null(false)?.execute::()`. `fill_null`'s +//! precondition calls `array.validity()?` on the lazy array, which re-evaluates the `LIKE` +//! predicate to derive validity (`Dict::validity` -> `ScalarFn::validity` -> `execute_expr`), +//! then materializes an intermediate array, after which `Mask::execute` canonicalizes again. +//! * `coercing_nulls` — `array.null_as_false().execute()`. Canonicalizes once, then folds +//! validity into the value bits with a single bitmap `AND`. +//! +//! `validity_lazy` vs `validity_canonical` isolates the single `.validity()` call that +//! `fill_null`'s precondition makes: O(predicate re-execution) on the lazy array vs O(1) on the +//! canonical one. + +#![expect(clippy::unwrap_used)] + +use divan::Bencher; +use vortex_array::ArrayRef; +use vortex_array::IntoArray; +use vortex_array::VortexSessionExecute; +use vortex_array::arrays::BoolArray; +use vortex_array::arrays::DictArray; +use vortex_array::arrays::PrimitiveArray; +use vortex_array::arrays::VarBinViewArray; +use vortex_array::builtins::ArrayBuiltins; +use vortex_array::expr::like; +use vortex_array::expr::lit; +use vortex_array::expr::root; +use vortex_mask::Mask; + +fn main() { + divan::main(); +} + +const LEN: usize = 65_536; + +/// Distinct-value counts mirroring dict cardinality: from a low-cardinality categorical column up +/// to a near-unique high-cardinality column such as ClickBench `URL`. The re-executed `LIKE` +/// validity scales with this count, so it is the parameter that drives the regression. +const CARDINALITIES: &[usize] = &[256, 4_096, 32_768, 65_536]; + +/// A lazy `Dict{codes, values: distinct.apply(like('%google%'))}` of nullable booleans, mirroring +/// the dict reader's `values.take(codes)` for a `LIKE` filter over a nullable string column. +fn lazy_dict_predicate(n_distinct: usize) -> ArrayRef { + // Distinct dictionary values: nullable URL-like strings, ~1/8 matching "%google%", 1/16 null. + let distinct = VarBinViewArray::from_iter_nullable_str((0..n_distinct).map(|i| { + if i % 16 == 0 { + None + } else if i % 8 == 0 { + Some(format!("http://www.google.com/path/{i}")) + } else { + Some(format!("http://example.com/page/{i}")) + } + })) + .into_array(); + + // Lazy predicate over the distinct values: not canonicalized. + let predicate = distinct.apply(&like(root(), lit("%google%"))).unwrap(); + + let codes = PrimitiveArray::from_iter((0..LEN).map(|i| (i % n_distinct) as u64)).into_array(); + + DictArray::try_new(codes, predicate).unwrap().into_array() +} + +/// A lazy `ScalarFn` array: `column.apply(like('%google%'))` over a full-length nullable string +/// column, mirroring the flat reader's `array.apply(&expr)`. Unlike the dict case, `ScalarFn`'s +/// `validity()` *executes* the predicate expression, so `fill_null`'s precondition pays for it. +fn lazy_scalar_fn_predicate() -> ArrayRef { + let column = VarBinViewArray::from_iter_nullable_str((0..LEN).map(|i| { + if i % 16 == 0 { + None + } else if i % 8 == 0 { + Some(format!("http://www.google.com/path/{i}")) + } else { + Some(format!("http://example.com/page/{i}")) + } + })) + .into_array(); + + column.apply(&like(root(), lit("%google%"))).unwrap() +} + +/// #8121 path: `fill_null(false)` on the lazy array, then `Mask::execute`. +#[divan::bench(args = CARDINALITIES)] +fn fill_null_then_mask(bencher: Bencher, n_distinct: usize) { + let array = lazy_dict_predicate(n_distinct); + let session = vortex_array::array_session(); + bencher + .with_inputs(|| (array.clone(), session.create_execution_ctx())) + .bench_values(|(array, mut ctx)| { + array + .fill_null(false) + .unwrap() + .execute::(&mut ctx) + .unwrap() + }); +} + +/// Fix: canonicalize once and fold validity into the bits. +#[divan::bench(args = CARDINALITIES)] +fn coercing_nulls(bencher: Bencher, n_distinct: usize) { + let array = lazy_dict_predicate(n_distinct); + let session = vortex_array::array_session(); + bencher + .with_inputs(|| (array.clone(), session.create_execution_ctx())) + .bench_values(|(array, mut ctx)| array.null_as_false().execute(&mut ctx).unwrap()); +} + +/// Isolation: the single `.validity()` call `fill_null`'s precondition makes, on the lazy array. +/// This re-executes the `LIKE` predicate to derive validity, so it scales with cardinality. +#[divan::bench(args = CARDINALITIES)] +fn validity_lazy(bencher: Bencher, n_distinct: usize) { + let array = lazy_dict_predicate(n_distinct); + bencher + .with_inputs(|| array.clone()) + .bench_values(|array| array.validity().unwrap()); +} + +/// Isolation: the same `.validity()` call once the array is already canonical — O(1) bitmap read. +#[divan::bench(args = CARDINALITIES)] +fn validity_canonical(bencher: Bencher, n_distinct: usize) { + let array = lazy_dict_predicate(n_distinct); + let mut ctx = vortex_array::array_session().create_execution_ctx(); + let canonical = array.execute::(&mut ctx).unwrap().into_array(); + bencher + .with_inputs(|| canonical.clone()) + .bench_values(|array| array.validity().unwrap()); +} + +// --- ScalarFn (flat reader) shape: this is the case `fill_null` handles badly. --- + +/// #8121 path on a `ScalarFn` array: `fill_null`'s precondition `.validity()` executes the predicate. +#[divan::bench] +fn scalar_fn_fill_null_then_mask(bencher: Bencher) { + let array = lazy_scalar_fn_predicate(); + let session = vortex_array::array_session(); + bencher + .with_inputs(|| (array.clone(), session.create_execution_ctx())) + .bench_values(|(array, mut ctx)| { + array + .fill_null(false) + .unwrap() + .execute::(&mut ctx) + .unwrap() + }); +} + +/// Fix on a `ScalarFn` array: canonicalize once, then fold validity. +#[divan::bench] +fn scalar_fn_coercing_nulls(bencher: Bencher) { + let array = lazy_scalar_fn_predicate(); + let session = vortex_array::array_session(); + bencher + .with_inputs(|| (array.clone(), session.create_execution_ctx())) + .bench_values(|(array, mut ctx)| array.null_as_false().execute(&mut ctx).unwrap()); +} + +/// Isolation: `.validity()` on a lazy `ScalarFn` array — executes the predicate expression. +#[divan::bench] +fn scalar_fn_validity_lazy(bencher: Bencher) { + let array = lazy_scalar_fn_predicate(); + bencher + .with_inputs(|| array.clone()) + .bench_values(|array| array.validity().unwrap()); +} 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 3d6317666d4..9fa5cecc6ba 100644 --- a/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/is_constant/mod.rs @@ -43,7 +43,6 @@ use crate::expr::stats::Precision; use crate::expr::stats::Stat; use crate::expr::stats::StatsProvider; use crate::expr::stats::StatsProviderExt; -use crate::mask::execute_mask_coercing_nulls; use crate::scalar::Scalar; use crate::scalar_fn::fns::operators::Operator; @@ -73,7 +72,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 = execute_mask_coercing_nulls(eq_result, ctx)?; + let eq_result = eq_result.null_as_false().execute(ctx)?; Ok(eq_result.true_count() == valid_count) } diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 99d43e4a7e3..1730adfd98a 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; @@ -15,6 +13,7 @@ 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`]. @@ -43,35 +42,58 @@ impl Executable for Mask { } } -/// Executes a boolean array into a [`Mask`], coercing null elements to `false` (SQL-style -/// `NULL`-as-not-matching semantics). +/// An adapter that coerces null elements of a boolean array to `false` before executing it into a +/// [`Mask`]. Created by [`ArrayRef::null_as_false`]. +/// +/// Use for filter and pruning predicates over nullable data, where SQL semantics treat `NULL` as +/// not matching. /// -/// This is the cheap counterpart to [`ArrayRef::fill_null(false)`](crate::builtins::ArrayBuiltins::fill_null) -/// followed by [`Mask::execute`]. It canonicalizes the (possibly lazy) array exactly once and folds -/// validity into the value bits with a single bitmap `AND`, rather than routing through the generic -/// `fill_null` scalar function, which derives validity from the lazy expression tree (re-evaluating -/// predicates such as `LIKE`) and materializes an intermediate array. -pub fn execute_mask_coercing_nulls(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult { - if !matches!(array.dtype(), DType::Bool(_)) { - vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); +/// Prefer `array.null_as_false().execute(ctx)` over `array.fill_null(false)?.execute::(ctx)`: +/// `fill_null` on a lazy `ScalarFn` array (e.g. the result of `apply()`) is currently +/// slow because its `validity()` executes the predicate expression. +pub struct NullAsFalse(ArrayRef); + +impl ArrayRef { + /// Returns an adapter that treats null elements of this boolean array as `false` when executed + /// into a [`Mask`]. See [`NullAsFalse`]. + pub fn null_as_false(self) -> NullAsFalse { + NullAsFalse(self) } +} - 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)) +impl NullAsFalse { + /// Executes the boolean array into a [`Mask`], coercing null elements to `false`. + /// + /// Canonicalizes the (possibly lazy) array exactly once and folds validity into the value bits + /// with a single `AND` that reuses the value buffer when it is uniquely owned. + pub fn execute(self, ctx: &mut ExecutionCtx) -> VortexResult { + let array = self.0; + if !matches!(array.dtype(), DType::Bool(_)) { + vortex_bail!("Mask array must have boolean dtype, not {}", array.dtype()); } - Columnar::Canonical(a) => { - let bool = a.into_array().execute::(ctx)?; - // Treat nulls as `false`: fold validity into the value bits. - let validity = bool - .as_ref() - .validity()? - .execute_mask(bool.as_ref().len(), ctx)?; - let bits = bool.into_bit_buffer(); - validity.bitand(&Mask::from(bits)) + // Non-nullable input needs no coercion; defer to the strict `Mask` execution. + 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() & &validity_bits) + } + } + } + }) + } } #[cfg(test)] @@ -88,7 +110,6 @@ mod tests { use crate::builtins::ArrayBuiltins; use crate::dtype::DType; use crate::dtype::Nullability; - use crate::mask::execute_mask_coercing_nulls; use crate::scalar::Scalar; fn ctx() -> ExecutionCtx { @@ -127,39 +148,39 @@ mod tests { } #[test] - fn coercing_nulls_non_nullable() -> VortexResult<()> { + fn null_as_false_non_nullable() -> VortexResult<()> { let array = BoolArray::from_iter([true, false, true]).into_array(); - let mask = execute_mask_coercing_nulls(array, &mut ctx())?; + let mask = array.null_as_false().execute(&mut ctx())?; assert_eq!(mask, Mask::from_iter([true, false, true])); Ok(()) } #[test] - fn coercing_nulls_treats_null_as_false() -> VortexResult<()> { + fn null_as_false_treats_null_as_false() -> VortexResult<()> { let array = BoolArray::from_iter([Some(true), None, Some(false), None]).into_array(); - let mask = execute_mask_coercing_nulls(array, &mut ctx())?; + let mask = array.null_as_false().execute(&mut ctx())?; assert_eq!(mask, Mask::from_iter([true, false, false, false])); Ok(()) } #[test] - fn coercing_nulls_null_constant() -> VortexResult<()> { + fn null_as_false_null_constant() -> VortexResult<()> { let array = ConstantArray::new(Scalar::null(DType::Bool(Nullability::Nullable)), 4).into_array(); - let mask = execute_mask_coercing_nulls(array, &mut ctx())?; + let mask = array.null_as_false().execute(&mut ctx())?; assert_eq!(mask, Mask::new_false(4)); Ok(()) } #[test] - fn coercing_nulls_matches_fill_null_then_mask() -> VortexResult<()> { + fn null_as_false_matches_fill_null_then_mask() -> VortexResult<()> { let array = BoolArray::from_iter([Some(true), None, Some(false), Some(true), None]).into_array(); let via_fill_null = array .clone() .fill_null(false)? .execute::(&mut ctx())?; - let via_coerce = execute_mask_coercing_nulls(array, &mut ctx())?; + let via_coerce = array.null_as_false().execute(&mut ctx())?; assert_eq!(via_coerce, via_fill_null); Ok(()) } diff --git a/vortex-cuda/src/layout.rs b/vortex-cuda/src/layout.rs index c6ddaece84b..ac16d858d2d 100644 --- a/vortex-cuda/src/layout.rs +++ b/vortex-cuda/src/layout.rs @@ -26,7 +26,6 @@ use vortex::array::expr::Expression; use vortex::array::expr::stats::Precision; use vortex::array::expr::stats::Stat; use vortex::array::expr::stats::StatsProvider; -use vortex::array::mask::execute_mask_coercing_nulls; use vortex::array::normalize::NormalizeOptions; use vortex::array::normalize::Operation; use vortex::array::serde::SerializeOptions; @@ -332,12 +331,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 = execute_mask_coercing_nulls(array, &mut ctx)?; + 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 = execute_mask_coercing_nulls(array, &mut ctx)?; + 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 3219afce46c..685444a61e5 100644 --- a/vortex-layout/src/layouts/dict/reader.rs +++ b/vortex-layout/src/layouts/dict/reader.rs @@ -26,7 +26,6 @@ use vortex_array::expr::label_tree; use vortex_array::expr::pack; use vortex_array::expr::root; use vortex_array::expr::transform::partition_annotations; -use vortex_array::mask::execute_mask_coercing_nulls; use vortex_array::optimizer::ArrayOptimizer; use vortex_array::scalar_fn::is_negative_cost; use vortex_error::VortexError; @@ -259,7 +258,7 @@ impl LayoutReader for DictReader { let mask = mask.await?; let mut ctx = session.create_execution_ctx(); - let dict_mask = execute_mask_coercing_nulls(values.take(codes)?, &mut ctx)?; + let dict_mask = values.take(codes)?.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 d2b53f56d4d..6881d3c814a 100644 --- a/vortex-layout/src/layouts/flat/reader.rs +++ b/vortex-layout/src/layouts/flat/reader.rs @@ -14,7 +14,6 @@ use vortex_array::VortexSessionExecute; use vortex_array::dtype::DType; use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; -use vortex_array::mask::execute_mask_coercing_nulls; use vortex_array::serde::SerializedArray; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -157,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 = execute_mask_coercing_nulls(array, &mut ctx)?; + 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 = execute_mask_coercing_nulls(array, &mut ctx)?; + 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 752d90d4532..76327a7ee5f 100644 --- a/vortex-layout/src/layouts/partitioned.rs +++ b/vortex-layout/src/layouts/partitioned.rs @@ -15,7 +15,6 @@ 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::execute_mask_coercing_nulls; use vortex_array::validity::Validity; use vortex_error::VortexError; use vortex_error::VortexResult; @@ -90,7 +89,10 @@ impl PartitionedExprEval

for PartitionedExpr

{ .into_array(); let mut ctx = session.create_execution_ctx(); - let root_mask = execute_mask_coercing_nulls(root_scope.apply(&self.root)?, &mut ctx)?; + let root_mask = root_scope + .apply(&self.root)? + .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 f28f1180592..fb826a1b401 100644 --- a/vortex-layout/src/layouts/row_idx/mod.rs +++ b/vortex-layout/src/layouts/row_idx/mod.rs @@ -31,7 +31,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::mask::execute_mask_coercing_nulls; use vortex_array::scalar::PValue; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -296,7 +295,7 @@ 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 = execute_mask_coercing_nulls(array.apply(&expr)?, &mut ctx)?; + let result_mask = array.apply(&expr)?.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 85acadbc7c1..789a6a74e8d 100644 --- a/vortex-layout/src/layouts/zoned/zone_map.rs +++ b/vortex-layout/src/layouts/zoned/zone_map.rs @@ -28,7 +28,6 @@ use vortex_array::expr::root; use vortex_array::expr::stats::Stat; use vortex_array::expr::traversal::NodeExt; use vortex_array::expr::traversal::Transformed; -use vortex_array::mask::execute_mask_coercing_nulls; use vortex_array::scalar::Scalar; use vortex_array::scalar_fn::EmptyOptions; use vortex_array::scalar_fn::ScalarFnVTableExt; @@ -124,12 +123,12 @@ impl ZoneMap { let applied = self.array.clone().into_array().apply(&predicate)?; if !contains_row_count(&applied) { - return execute_mask_coercing_nulls(applied, &mut ctx); + return 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)?; - execute_mask_coercing_nulls(substituted, &mut ctx) + substituted.null_as_false().execute(&mut ctx) } fn lower_stats(&self, predicate: Expression) -> VortexResult { From 4ffba7ddc20564c7b29bfaa845b49519ed226363 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Wed, 17 Jun 2026 15:05:05 +0100 Subject: [PATCH 3/4] Remove mask_fill_null microbenchmark Signed-off-by: Joe Isaacs Co-Authored-By: Claude Opus 4.8 (1M context) --- vortex-array/Cargo.toml | 4 - vortex-array/benches/mask_fill_null.rs | 172 ------------------------- 2 files changed, 176 deletions(-) delete mode 100644 vortex-array/benches/mask_fill_null.rs diff --git a/vortex-array/Cargo.toml b/vortex-array/Cargo.toml index 1886c92268f..de3bb416985 100644 --- a/vortex-array/Cargo.toml +++ b/vortex-array/Cargo.toml @@ -101,10 +101,6 @@ vortex-array = { path = ".", features = ["_test-harness", "table-display"] } name = "aggregate_max" harness = false -[[bench]] -name = "mask_fill_null" -harness = false - [[bench]] name = "aggregate_sum" harness = false diff --git a/vortex-array/benches/mask_fill_null.rs b/vortex-array/benches/mask_fill_null.rs deleted file mode 100644 index 443e04c3148..00000000000 --- a/vortex-array/benches/mask_fill_null.rs +++ /dev/null @@ -1,172 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: Copyright the Vortex contributors - -//! Microbenchmark isolating why `fill_null(false)` + `Mask::execute` is slow on a *lazy*, -//! nullable predicate result (the shape produced by the dict layout reader for a filter such as -//! `URL LIKE '%google%'` over a nullable, dictionary-encoded string column). -//! -//! The lazy array is `Dict{ codes, values: distinct.apply(like) }`. Executing it into a `Mask` -//! has two strategies: -//! -//! * `fill_null_then_mask` — `array.fill_null(false)?.execute::()`. `fill_null`'s -//! precondition calls `array.validity()?` on the lazy array, which re-evaluates the `LIKE` -//! predicate to derive validity (`Dict::validity` -> `ScalarFn::validity` -> `execute_expr`), -//! then materializes an intermediate array, after which `Mask::execute` canonicalizes again. -//! * `coercing_nulls` — `array.null_as_false().execute()`. Canonicalizes once, then folds -//! validity into the value bits with a single bitmap `AND`. -//! -//! `validity_lazy` vs `validity_canonical` isolates the single `.validity()` call that -//! `fill_null`'s precondition makes: O(predicate re-execution) on the lazy array vs O(1) on the -//! canonical one. - -#![expect(clippy::unwrap_used)] - -use divan::Bencher; -use vortex_array::ArrayRef; -use vortex_array::IntoArray; -use vortex_array::VortexSessionExecute; -use vortex_array::arrays::BoolArray; -use vortex_array::arrays::DictArray; -use vortex_array::arrays::PrimitiveArray; -use vortex_array::arrays::VarBinViewArray; -use vortex_array::builtins::ArrayBuiltins; -use vortex_array::expr::like; -use vortex_array::expr::lit; -use vortex_array::expr::root; -use vortex_mask::Mask; - -fn main() { - divan::main(); -} - -const LEN: usize = 65_536; - -/// Distinct-value counts mirroring dict cardinality: from a low-cardinality categorical column up -/// to a near-unique high-cardinality column such as ClickBench `URL`. The re-executed `LIKE` -/// validity scales with this count, so it is the parameter that drives the regression. -const CARDINALITIES: &[usize] = &[256, 4_096, 32_768, 65_536]; - -/// A lazy `Dict{codes, values: distinct.apply(like('%google%'))}` of nullable booleans, mirroring -/// the dict reader's `values.take(codes)` for a `LIKE` filter over a nullable string column. -fn lazy_dict_predicate(n_distinct: usize) -> ArrayRef { - // Distinct dictionary values: nullable URL-like strings, ~1/8 matching "%google%", 1/16 null. - let distinct = VarBinViewArray::from_iter_nullable_str((0..n_distinct).map(|i| { - if i % 16 == 0 { - None - } else if i % 8 == 0 { - Some(format!("http://www.google.com/path/{i}")) - } else { - Some(format!("http://example.com/page/{i}")) - } - })) - .into_array(); - - // Lazy predicate over the distinct values: not canonicalized. - let predicate = distinct.apply(&like(root(), lit("%google%"))).unwrap(); - - let codes = PrimitiveArray::from_iter((0..LEN).map(|i| (i % n_distinct) as u64)).into_array(); - - DictArray::try_new(codes, predicate).unwrap().into_array() -} - -/// A lazy `ScalarFn` array: `column.apply(like('%google%'))` over a full-length nullable string -/// column, mirroring the flat reader's `array.apply(&expr)`. Unlike the dict case, `ScalarFn`'s -/// `validity()` *executes* the predicate expression, so `fill_null`'s precondition pays for it. -fn lazy_scalar_fn_predicate() -> ArrayRef { - let column = VarBinViewArray::from_iter_nullable_str((0..LEN).map(|i| { - if i % 16 == 0 { - None - } else if i % 8 == 0 { - Some(format!("http://www.google.com/path/{i}")) - } else { - Some(format!("http://example.com/page/{i}")) - } - })) - .into_array(); - - column.apply(&like(root(), lit("%google%"))).unwrap() -} - -/// #8121 path: `fill_null(false)` on the lazy array, then `Mask::execute`. -#[divan::bench(args = CARDINALITIES)] -fn fill_null_then_mask(bencher: Bencher, n_distinct: usize) { - let array = lazy_dict_predicate(n_distinct); - let session = vortex_array::array_session(); - bencher - .with_inputs(|| (array.clone(), session.create_execution_ctx())) - .bench_values(|(array, mut ctx)| { - array - .fill_null(false) - .unwrap() - .execute::(&mut ctx) - .unwrap() - }); -} - -/// Fix: canonicalize once and fold validity into the bits. -#[divan::bench(args = CARDINALITIES)] -fn coercing_nulls(bencher: Bencher, n_distinct: usize) { - let array = lazy_dict_predicate(n_distinct); - let session = vortex_array::array_session(); - bencher - .with_inputs(|| (array.clone(), session.create_execution_ctx())) - .bench_values(|(array, mut ctx)| array.null_as_false().execute(&mut ctx).unwrap()); -} - -/// Isolation: the single `.validity()` call `fill_null`'s precondition makes, on the lazy array. -/// This re-executes the `LIKE` predicate to derive validity, so it scales with cardinality. -#[divan::bench(args = CARDINALITIES)] -fn validity_lazy(bencher: Bencher, n_distinct: usize) { - let array = lazy_dict_predicate(n_distinct); - bencher - .with_inputs(|| array.clone()) - .bench_values(|array| array.validity().unwrap()); -} - -/// Isolation: the same `.validity()` call once the array is already canonical — O(1) bitmap read. -#[divan::bench(args = CARDINALITIES)] -fn validity_canonical(bencher: Bencher, n_distinct: usize) { - let array = lazy_dict_predicate(n_distinct); - let mut ctx = vortex_array::array_session().create_execution_ctx(); - let canonical = array.execute::(&mut ctx).unwrap().into_array(); - bencher - .with_inputs(|| canonical.clone()) - .bench_values(|array| array.validity().unwrap()); -} - -// --- ScalarFn (flat reader) shape: this is the case `fill_null` handles badly. --- - -/// #8121 path on a `ScalarFn` array: `fill_null`'s precondition `.validity()` executes the predicate. -#[divan::bench] -fn scalar_fn_fill_null_then_mask(bencher: Bencher) { - let array = lazy_scalar_fn_predicate(); - let session = vortex_array::array_session(); - bencher - .with_inputs(|| (array.clone(), session.create_execution_ctx())) - .bench_values(|(array, mut ctx)| { - array - .fill_null(false) - .unwrap() - .execute::(&mut ctx) - .unwrap() - }); -} - -/// Fix on a `ScalarFn` array: canonicalize once, then fold validity. -#[divan::bench] -fn scalar_fn_coercing_nulls(bencher: Bencher) { - let array = lazy_scalar_fn_predicate(); - let session = vortex_array::array_session(); - bencher - .with_inputs(|| (array.clone(), session.create_execution_ctx())) - .bench_values(|(array, mut ctx)| array.null_as_false().execute(&mut ctx).unwrap()); -} - -/// Isolation: `.validity()` on a lazy `ScalarFn` array — executes the predicate expression. -#[divan::bench] -fn scalar_fn_validity_lazy(bencher: Bencher) { - let array = lazy_scalar_fn_predicate(); - bencher - .with_inputs(|| array.clone()) - .bench_values(|array| array.validity().unwrap()); -} From 1138a8c98cfb45ad83e9b5e4991b8bca88a2b09c Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Wed, 17 Jun 2026 14:57:31 +0000 Subject: [PATCH 4/4] fix: remove redundant clone in mask test Clippy redundant_clone fired on array.clone() in null_as_false_matches_fill_null_then_mask since fill_null borrows self and the array is reused afterwards. Signed-off-by: Joe Isaacs --- vortex-array/src/mask.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/vortex-array/src/mask.rs b/vortex-array/src/mask.rs index 1730adfd98a..d8ba622f347 100644 --- a/vortex-array/src/mask.rs +++ b/vortex-array/src/mask.rs @@ -176,10 +176,7 @@ mod tests { fn null_as_false_matches_fill_null_then_mask() -> VortexResult<()> { let array = BoolArray::from_iter([Some(true), None, Some(false), Some(true), None]).into_array(); - let via_fill_null = array - .clone() - .fill_null(false)? - .execute::(&mut ctx())?; + let via_fill_null = array.fill_null(false)?.execute::(&mut ctx())?; let via_coerce = array.null_as_false().execute(&mut ctx())?; assert_eq!(via_coerce, via_fill_null); Ok(())