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
68 changes: 66 additions & 2 deletions datafusion/sql/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ use datafusion_expr::{
};

use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
use datafusion_functions_nested::expr_fn::{array_has, array_max, array_min};
use datafusion_functions_nested::expr_fn::{
array_has, array_max, array_min, array_position, cardinality,
};

mod binary_op;
mod function;
Expand Down Expand Up @@ -635,7 +637,11 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
schema,
planner_context,
),
_ => not_impl_err!("ALL only supports subquery comparison currently"),
_ => {
let left_expr = self.sql_to_expr(*left, schema, planner_context)?;
let right_expr = self.sql_to_expr(*right, schema, planner_context)?;
plan_all_op(&left_expr, &right_expr, &compare_op)
}
},
#[expect(deprecated)]
SQLExpr::Wildcard(_token) => Ok(Expr::Wildcard {
Expand Down Expand Up @@ -1297,6 +1303,64 @@ fn plan_any_op(
}
}

/// Plans `needle <compare_op> ALL(haystack)` with proper SQL NULL semantics.
///
/// CASE/WHEN structure:
/// WHEN arr IS NULL → NULL
/// WHEN empty → TRUE
/// WHEN lhs IS NULL → NULL
/// WHEN decisive_condition → FALSE
/// WHEN has_nulls → NULL
/// ELSE → TRUE
fn plan_all_op(
needle: &Expr,
haystack: &Expr,
compare_op: &BinaryOperator,
) -> Result<Expr> {
let null_arr_check = haystack.clone().is_null();
let empty_check = cardinality(haystack.clone()).eq(lit(0u64));
let null_lhs_check = needle.clone().is_null();
// DataFusion's array_position uses is_null() checks internally (not equality),
// so it can locate NULL elements even though NULL = NULL is NULL in standard SQL.
let has_nulls =
array_position(haystack.clone(), lit(ScalarValue::Null), lit(1i64)).is_not_null();

let decisive_condition = match compare_op {
BinaryOperator::NotEq => array_has(haystack.clone(), needle.clone()),
BinaryOperator::Eq => {
let all_equal = array_min(haystack.clone())
.eq(needle.clone())
.and(array_max(haystack.clone()).eq(needle.clone()));
Expr::Not(Box::new(all_equal))
}
BinaryOperator::Gt => {
Expr::Not(Box::new(needle.clone().gt(array_max(haystack.clone()))))
}
BinaryOperator::Lt => {
Expr::Not(Box::new(needle.clone().lt(array_min(haystack.clone()))))
}
BinaryOperator::GtEq => {
Expr::Not(Box::new(needle.clone().gt_eq(array_max(haystack.clone()))))
}
BinaryOperator::LtEq => {
Expr::Not(Box::new(needle.clone().lt_eq(array_min(haystack.clone()))))
}
_ => {
return plan_err!(
"Unsupported AllOp: '{compare_op}', only '=', '<>', '>', '<', '>=', '<=' are supported"
);
}
};

let null_bool = lit(ScalarValue::Boolean(None));
when(null_arr_check, null_bool.clone())
.when(empty_check, lit(true))
.when(null_lhs_check, null_bool.clone())
.when(decisive_condition, lit(false))
.when(has_nulls, null_bool)
.otherwise(lit(true))
Comment on lines +1356 to +1361
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.

I do wonder if we're better off having a UDF implementation instead of this case approach, though maybe thats something that can be explored as a followup

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was thinking of this as well especially regarding performance wise. As you've said I thought maybe I can have a followup UDF implementation and compare results

}

#[cfg(test)]
mod tests {
use std::collections::HashMap;
Expand Down
221 changes: 221 additions & 0 deletions datafusion/sqllogictest/test_files/array/array_all.slt
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

## all operator

# = ALL: true when all elements equal val
query B
select 5 = ALL(make_array(5, 5, 5));
----
true

query B
select 5 = ALL(make_array(5, 5, 3));
----
false

# <> ALL: true when val differs from every element
query B
select 5 <> ALL(make_array(1, 2, 3));
----
true

query B
select 5 <> ALL(make_array(1, 2, 5));
----
false

# > ALL: true when val greater than all elements
query B
select 10 > ALL(make_array(1, 2, 3));
----
true

query B
select 3 > ALL(make_array(1, 2, 3));
----
false

# < ALL: true when val less than all elements
query B
select 0 < ALL(make_array(1, 2, 3));
----
true

query B
select 2 < ALL(make_array(1, 2, 3));
----
false

# >= ALL: true when val >= all elements
query B
select 5 >= ALL(make_array(1, 2, 5));
----
true

query B
select 4 >= ALL(make_array(1, 2, 5));
----
false

# <= ALL: true when val <= all elements
query B
select 1 <= ALL(make_array(1, 2, 5));
----
true

query B
select 2 <= ALL(make_array(1, 2, 5));
----
false

# Empty arrays: all operators return TRUE (vacuous truth)
query B
select 5 = ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true

query B
select 5 <> ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true

query B
select 5 > ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true

query B
select 5 < ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true

query B
select 5 >= ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true

query B
select 5 <= ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true

# NULL LHS with empty array returns TRUE (vacuous truth)
query B
select NULL = ALL(arrow_cast(make_array(), 'List(Int64)'));
----
true
Comment on lines +117 to +121
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.

I find this a bit surprising since I would assume if the needle is null then we'd always return null regardless of the haystack 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was surprised about the semantics and results as well btw. That's why I've checked every edge case with postgres and continued with their approach but you're right


# NULL LHS with non-empty array returns NULL
query B
select NULL = ALL(make_array(1, 2, 3));
----
NULL

query B
select NULL > ALL(make_array(1, 2, 3));
----
NULL

query B
select NULL <> ALL(make_array(1, 2, 3));
----
NULL

# All-NULL arrays: returns NULL
query B
select 5 = ALL(make_array(NULL::INT, NULL::INT));
----
NULL

query B
select 5 <> ALL(make_array(NULL::INT, NULL::INT));
----
NULL

query B
select 5 > ALL(make_array(NULL::INT, NULL::INT));
----
NULL

query B
select 5 < ALL(make_array(NULL::INT, NULL::INT));
----
NULL

query B
select 5 >= ALL(make_array(NULL::INT, NULL::INT));
----
NULL

query B
select 5 <= ALL(make_array(NULL::INT, NULL::INT));
----
NULL

# Mixed NULL + non-NULL (non-NULL elements satisfy, but NULLs present → NULL)
query B
select 5 > ALL(make_array(3, NULL));
----
NULL

query B
select 5 >= ALL(make_array(5, NULL));
----
NULL

query B
select 1 < ALL(make_array(3, NULL));
----
NULL

query B
select 1 <= ALL(make_array(1, NULL));
----
NULL

# Mixed NULL + non-NULL (not satisfying condition → FALSE wins over NULL)
query B
select 5 > ALL(make_array(6, NULL));
----
false

query B
select 5 < ALL(make_array(3, NULL));
----
false

query B
select 5 = ALL(make_array(5, 3, NULL));
----
false

# NULL array input returns NULL
query B
select 5 = ALL(NULL::INT[]);
----
NULL

query B
select 5 > ALL(NULL::INT[]);
----
NULL

query B
select 5 < ALL(NULL::INT[]);
----
NULL
Loading