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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/concepts/expressions.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ to the returned data.
## Scalar Functions

Expressions are defined as abstract scalar functions. These vtables define the signature of the function, properties
such as whether the function is null-sensitive, and the actual logic for executing the function over input arrays.
such as whether the function is strict, and the actual logic for executing the function over input arrays.

The built-in scalar functions can be found in the `vortex-array::expr` module, with additional use-case specific
functions provided by integration and plugin crates.
Expand Down
2 changes: 1 addition & 1 deletion encodings/parquet-variant/src/json_to_variant_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ fn converts_json_extension_input() -> VortexResult<()> {
#[test]
fn dict_encoded_input_converts_each_row() -> VortexResult<()> {
// A dictionary-encoded JSON column exercises the dict-pushdown / canonicalization path:
// `json_to_variant` is not null-sensitive, so it pushes into the dict values (canonical
// `json_to_variant` is strict, so it pushes into the dict values (canonical
// JSON extension values) where the kernel fires; either way every row must convert correctly.
let values = json_input(VarBinViewArray::from_iter_str(["1", "2"]).into_array())?;
let codes = PrimitiveArray::from_iter([0u8, 1, 0, 1, 0]).into_array();
Expand Down
18 changes: 9 additions & 9 deletions vortex-array/src/arrays/dict/compute/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,18 @@ impl ArrayParentReduceRule<Dict> for DictionaryScalarFnValuesPushDownRule {
return Ok(None);
}

// If the scalar function is null-sensitive, then we cannot push it down to values if
// we have any nulls in the codes.
// Before this rewrite, a null code supplies null for this argument while the constant
// arguments retain their values. After the rewrite, the null code masks the function's
// result. Those are equivalent only for a strict function.
if array.codes().dtype().is_nullable()
&& !matches!(
array.codes().validity()?,
Validity::NonNullable | Validity::AllValid
)
&& sig.is_null_sensitive()
&& !sig.is_strict()
{
tracing::trace!(
"Not pushing down null-sensitive scalar function {} over dictionary with null codes {}",
"Not pushing down non-strict scalar function {} over dictionary with null codes {}",
parent.scalar_fn(),
Dict.id(),
);
Expand All @@ -182,11 +183,10 @@ impl ArrayParentReduceRule<Dict> for DictionaryScalarFnValuesPushDownRule {
.into_array()
.optimize()?;

// We can only push down null-sensitive functions when we have all-valid codes.
// In these cases, we cannot have the codes influence the nullability of the output DType.
// Therefore, we cast the codes to be non-nullable and then cast the dictionary output
// back to nullable if needed.
if sig.is_null_sensitive() && array.codes().dtype().is_nullable() {
// A non-strict function reaches this point only when the codes are all valid, but their
// dtype may still be nullable. Remove that declared nullability while rebuilding the
// dictionary, then cast its output to the function's declared dtype.
if !sig.is_strict() && array.codes().dtype().is_nullable() {
let new_codes = array.codes().cast(array.codes().dtype().as_nonnullable())?;
let new_dict = unsafe {
DictArray::new_unchecked(new_codes, new_values)
Expand Down
4 changes: 4 additions & 0 deletions vortex-array/src/arrays/scalar_fn/vtable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,4 +337,8 @@ impl scalar_fn::ScalarFnVTable for ArrayExpr {
let validity_array = options.0.validity()?.to_array(options.0.len());
Ok(Some(ArrayExpr.new_expr(FakeEq(validity_array), [])))
}

fn is_strict(&self, _options: &Self::Options) -> bool {
true
}
Comment on lines +341 to +343

@joseph-isaacs joseph-isaacs Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is not the default, is it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I changed the default to false on the vtable?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add this file is the wrong vtable lol

}
3 changes: 3 additions & 0 deletions vortex-array/src/expr/analysis/labeling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ use crate::expr::traversal::NodeExt;
use crate::expr::traversal::NodeVisitor;
use crate::expr::traversal::TraversalOrder;

/// Boolean labels keyed by each expression node in a tree.
pub type BooleanLabels<'a> = HashMap<&'a Expression, bool>;

/// Label each node in an expression tree using a bottom-up traversal.
///
/// This function separates tree labeling into two distinct steps:
Expand Down
5 changes: 2 additions & 3 deletions vortex-array/src/expr/analysis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ pub mod annotation;
mod fallible;
pub mod immediate_access;
mod labeling;
mod null_sensitive;
mod referenced_field_paths;
mod strict;

pub use annotation::*;
pub use fallible::label_is_fallible;
pub use immediate_access::*;
pub use labeling::*;
pub use null_sensitive::BooleanLabels;
pub use null_sensitive::label_null_sensitive;
pub use referenced_field_paths::referenced_field_paths;
pub use strict::label_strict;
62 changes: 0 additions & 62 deletions vortex-array/src/expr/analysis/null_sensitive.rs

This file was deleted.

56 changes: 56 additions & 0 deletions vortex-array/src/expr/analysis/strict.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright the Vortex contributors

use super::BooleanLabels;
use super::labeling::label_tree;
use crate::expr::Expression;

/// Label each expression with whether its entire subtree is strict.
///
/// A subtree is strict only when the node's scalar function and every child subtree are strict.
/// See [`crate::scalar_fn::ScalarFnVTable::is_strict`] for the scalar-function contract.
pub fn label_strict(expr: &Expression) -> BooleanLabels<'_> {
label_tree(
expr,
|expr| expr.signature().is_strict(),
|acc, &child| acc & child,
)
}

#[cfg(test)]
mod tests {
use super::*;
use crate::expr::col;
use crate::expr::eq;
use crate::expr::is_null;
use crate::expr::lit;

#[test]
fn test_non_strict_with_is_null() {
let expr = is_null(col("col1"));
let labels = label_strict(&expr);

assert_eq!(labels.get(&expr), Some(&false));
}

#[test]
fn test_strict_expression() {
let expr = eq(lit(4), lit(5));
let labels = label_strict(&expr);

assert_eq!(labels.get(&expr), Some(&true));
}

#[test]
fn test_non_strict_child_makes_parent_subtree_non_strict() {
let left = eq(lit(4), lit(5));
let right = is_null(col("col2"));
let expr = eq(left.clone(), right.clone());

let labels = label_strict(&expr);

assert_eq!(labels.get(&left), Some(&true));
assert_eq!(labels.get(&right), Some(&false));
assert_eq!(labels.get(&expr), Some(&false));
}
}
2 changes: 1 addition & 1 deletion vortex-array/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//!
//! Each node references a scalar function defined by a
//! [`ScalarFnVTable`](crate::scalar_fn::ScalarFnVTable). The vtable declares the function signature,
//! properties such as null-sensitivity, and the logic that executes it over input arrays. Built-in
//! properties such as strictness, and the logic that executes it over input arrays. Built-in
//! functions live in [`crate::scalar_fn`]; integration and plugin crates supply additional,
//! use-case-specific functions.
//!
Expand Down
17 changes: 16 additions & 1 deletion vortex-array/src/scalar_fn/fns/between/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ impl ScalarFnVTable for Between {
Ok(Some(and(and(arr, lower), upper)))
}

fn is_null_sensitive(&self, _instance: &Self::Options) -> bool {
fn is_strict(&self, _options: &Self::Options) -> bool {
false
}

Expand Down Expand Up @@ -343,6 +343,21 @@ mod tests {

static SESSION: LazyLock<VortexSession> = LazyLock::new(crate::array_session);

#[test]
fn is_not_strict() {
let expr = between(
root(),
lit(0),
lit(100),
BetweenOptions {
lower_strict: StrictComparison::NonStrict,
upper_strict: StrictComparison::NonStrict,
},
);

assert!(!expr.signature().is_strict());
}

#[test]
fn test_display() {
let expr = between(
Expand Down
3 changes: 2 additions & 1 deletion vortex-array/src/scalar_fn/fns/binary/boolean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ use crate::validity::Validity;
/// operand without falling back to ordinary execution.
///
/// Vortex's boolean [`Operator::And`] and [`Operator::Or`] variants use Kleene semantics; there is
/// no separate two-valued boolean operator path to dispatch here.
/// no separate two-valued boolean operator path to dispatch here. Consequently, they are not
/// strict: `false AND null` is `false`, and `true OR null` is `true`.
pub trait BooleanKernel: VTable {
/// Execute `lhs <operator> rhs` using Kleene boolean semantics.
fn boolean(
Expand Down
9 changes: 4 additions & 5 deletions vortex-array/src/scalar_fn/fns/binary/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,10 @@ impl ScalarFnVTable for Binary {
})
}

fn is_null_sensitive(&self, operator: &Operator) -> bool {
// Kleene AND/OR is not strict (`false AND null = false`, `true OR null = true`), so
// these operators cannot be pushed through dictionary null codes. This is consistent
// with `validity` returning `None` for AND/OR above.
matches!(operator, Operator::And | Operator::Or)
fn is_strict(&self, operator: &Operator) -> bool {
// Kleene AND/OR is not strict (`false AND null = false`, `true OR null = true`), which is
// consistent with `validity` returning `None` for these operators above.
!matches!(operator, Operator::And | Operator::Or)
}

fn is_fallible(&self, operator: &Operator) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/scalar_fn/fns/byte_length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ impl ScalarFnVTable for ByteLength {
Ok(Some(expression.child(0).validity()?))
}

fn is_null_sensitive(&self, _options: &Self::Options) -> bool {
false
fn is_strict(&self, _options: &Self::Options) -> bool {
true
}

fn is_fallible(&self, _options: &Self::Options) -> bool {
Expand Down
5 changes: 3 additions & 2 deletions vortex-array/src/scalar_fn/fns/case_when.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,9 @@ impl ScalarFnVTable for CaseWhen {
Ok(Some(crate::expr::fill_null(x.clone(), fill.clone())))
}

fn is_null_sensitive(&self, _options: &Self::Options) -> bool {
true
fn is_strict(&self, _options: &Self::Options) -> bool {
// A null in an unselected branch does not force a null output.
false
}

fn is_fallible(&self, _options: &Self::Options) -> bool {
Expand Down
6 changes: 3 additions & 3 deletions vortex-array/src/scalar_fn/fns/cast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ impl ScalarFnVTable for Cast {
}))
}

// This might apply a nullability
fn is_null_sensitive(&self, _instance: &DType) -> bool {
true
fn is_strict(&self, _instance: &DType) -> bool {
// Cast options can pin a non-nullable output dtype instead of propagating nullability.
false
}
}

Expand Down
17 changes: 15 additions & 2 deletions vortex-array/src/scalar_fn/fns/dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ impl ScalarFnVTable for DynamicComparison {
.into_array())
}

// Defer to the child
fn is_null_sensitive(&self, _instance: &Self::Options) -> bool {
fn is_strict(&self, _options: &Self::Options) -> bool {
false
}
}
Expand Down Expand Up @@ -286,6 +285,20 @@ mod tests {
use crate::dtype::PType;
use crate::expr::dynamic;
use crate::expr::root;

#[test]
fn is_not_strict() {
let expr = dynamic(
CompareOperator::Lt,
|| None,
DType::Primitive(PType::I32, Nullability::NonNullable),
true,
root(),
);

assert!(!expr.signature().is_strict());
}

#[test]
fn return_dtype_bool() -> VortexResult<()> {
let expr = dynamic(
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/scalar_fn/fns/ext_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ impl ScalarFnVTable for ExtStorage {
Ok(Some(expression.child(0).validity()?))
}

fn is_null_sensitive(&self, _options: &Self::Options) -> bool {
false
fn is_strict(&self, _options: &Self::Options) -> bool {
true
}

fn is_fallible(&self, _options: &Self::Options) -> bool {
Expand Down
5 changes: 3 additions & 2 deletions vortex-array/src/scalar_fn/fns/fill_null/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ impl ScalarFnVTable for FillNull {
Ok(Some(expression.child(1).validity()?))
}

fn is_null_sensitive(&self, _options: &Self::Options) -> bool {
true
fn is_strict(&self, _options: &Self::Options) -> bool {
// This function replaces null input values instead of propagating them.
false
}

fn is_fallible(&self, _options: &Self::Options) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/scalar_fn/fns/get_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ impl ScalarFnVTable for GetItem {
Ok(None)
}

// This will apply struct nullability field. We could add a dtype??
fn is_null_sensitive(&self, _field_name: &FieldName) -> bool {
fn is_strict(&self, _field_name: &FieldName) -> bool {
true
}

Expand Down Expand Up @@ -227,6 +226,7 @@ mod tests {
fn get_item_by_name() {
let st = test_array();
let get_item = get_item("a", root());
assert!(get_item.signature().is_strict());
let item = st.into_array().apply(&get_item).unwrap();
assert_eq!(item.dtype(), &DType::from(PType::I32))
}
Expand Down
Loading
Loading