This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-22345-c8b784a01f5d0bcbe0dac806730fb61afc0be8ef in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 74fa029f4b38c4ab8bfb1b4d3ae2d149d1fa975f Author: Andrew Lamb <[email protected]> AuthorDate: Tue May 19 12:04:49 2026 -0400 Revert "[Minor]: unify ANY/ALL planning and align ANY NULL semantics with PG (#21743)" (#22345) ## Which issue does this PR close? - Closes https://github.com/apache/datafusion/issues/22073 ## Rationale for this change As @cetra3 describes on https://github.com/apache/datafusion/issues/22073, in https://github.com/apache/datafusion/pull/21743 @berkaysynnada made a seemingly small change to align to postgres semantics that had much larger consequences I think many users (for example @cetra3) will treat this change as a regression, and thus I think this issue would block the next datafusion release - https://github.com/apache/datafusion/issues/21080 We tried an alternate fix but it appears to also raise some subtle questions from @Jefffrey - https://github.com/apache/datafusion/pull/22102#issuecomment-4451305175 Thus I think it is safest to revert the initial change and work out the better longer term fix before releasing ## What changes are included in this PR? - Revert https://github.com/apache/datafusion/pull/21743 / 4bff17ebd2394e319b4adee77df18a011116ff2d until we can have a beter ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> --- datafusion/sql/src/expr/mod.rs | 122 ++++++++++++--------- datafusion/sql/tests/cases/plan_to_sql.rs | 2 +- .../sqllogictest/test_files/array/array_has.slt | 70 +++--------- 3 files changed, 90 insertions(+), 104 deletions(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index daf092ecd4..ba7811acd8 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -621,12 +621,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> { _ => { let left_expr = self.sql_to_expr(*left, schema, planner_context)?; let right_expr = self.sql_to_expr(*right, schema, planner_context)?; - plan_quantified_op( - &left_expr, - &right_expr, - &compare_op, - SetQuantifier::Any, - ) + plan_any_op(left_expr, right_expr, &compare_op) } }, SQLExpr::AllOp { @@ -645,12 +640,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> { _ => { let left_expr = self.sql_to_expr(*left, schema, planner_context)?; let right_expr = self.sql_to_expr(*right, schema, planner_context)?; - plan_quantified_op( - &left_expr, - &right_expr, - &compare_op, - SetQuantifier::All, - ) + plan_all_op(&left_expr, &right_expr, &compare_op) } }, #[expect(deprecated)] @@ -1259,20 +1249,73 @@ impl<S: ContextProvider> SqlToRel<'_, S> { } } -/// Plans `needle <compare_op> ANY/ALL(haystack)` with proper SQL NULL semantics. +/// Builds a CASE expression that handles NULL semantics for `x <op> ANY(arr)`: +/// +/// ```text +/// CASE +/// WHEN <min_or_max>(arr) IS NOT NULL THEN <comparison> +/// WHEN arr IS NOT NULL THEN FALSE -- empty or all-null array +/// ELSE NULL -- NULL array +/// END +/// ``` +fn any_op_with_null_handling(bound: Expr, comparison: Expr, arr: Expr) -> Result<Expr> { + when(bound.is_not_null(), comparison) + .when(arr.is_not_null(), lit(false)) + .otherwise(lit(ScalarValue::Boolean(None))) +} + +/// Plans a `<left> <op> ANY(<right>)` expression for non-subquery operands. +fn plan_any_op( + left_expr: Expr, + right_expr: Expr, + compare_op: &BinaryOperator, +) -> Result<Expr> { + match compare_op { + BinaryOperator::Eq => Ok(array_has(right_expr, left_expr)), + BinaryOperator::NotEq => { + let min = array_min(right_expr.clone()); + let max = array_max(right_expr.clone()); + // NOT EQ is true when either bound differs from left + let comparison = min + .not_eq(left_expr.clone()) + .or(max.clone().not_eq(left_expr)); + any_op_with_null_handling(max, comparison, right_expr) + } + BinaryOperator::Gt => { + let min = array_min(right_expr.clone()); + any_op_with_null_handling(min.clone(), min.lt(left_expr), right_expr) + } + BinaryOperator::Lt => { + let max = array_max(right_expr.clone()); + any_op_with_null_handling(max.clone(), max.gt(left_expr), right_expr) + } + BinaryOperator::GtEq => { + let min = array_min(right_expr.clone()); + any_op_with_null_handling(min.clone(), min.lt_eq(left_expr), right_expr) + } + BinaryOperator::LtEq => { + let max = array_max(right_expr.clone()); + any_op_with_null_handling(max.clone(), max.gt_eq(left_expr), right_expr) + } + _ => plan_err!( + "Unsupported AnyOp: '{compare_op}', only '=', '<>', '>', '<', '>=', '<=' are supported" + ), + } +} + +/// Plans `needle <compare_op> ALL(haystack)` with proper SQL NULL semantics. /// /// CASE/WHEN structure: /// WHEN arr IS NULL → NULL -/// WHEN empty → vacuous_result (ANY:false, ALL:true) +/// WHEN empty → TRUE /// WHEN lhs IS NULL → NULL -/// WHEN decisive_condition → decisive_result (ANY:true match found, ALL:false violation found) +/// WHEN decisive_condition → FALSE /// WHEN has_nulls → NULL -/// ELSE → vacuous_result -fn plan_quantified_op( +/// ELSE → TRUE +fn plan_all_op( needle: &Expr, haystack: &Expr, compare_op: &BinaryOperator, - quantifier: SetQuantifier, ) -> Result<Expr> { let null_arr_check = haystack.clone().is_null(); let empty_check = cardinality(haystack.clone()).eq(lit(0u64)); @@ -1282,61 +1325,40 @@ fn plan_quantified_op( let has_nulls = array_position(haystack.clone(), lit(ScalarValue::Null), lit(1i64)).is_not_null(); - let decisive_condition = match (compare_op, quantifier) { - (BinaryOperator::Eq, SetQuantifier::Any) - | (BinaryOperator::NotEq, SetQuantifier::All) => { - array_has(haystack.clone(), needle.clone()) - } - (BinaryOperator::Eq, SetQuantifier::All) - | (BinaryOperator::NotEq, SetQuantifier::Any) => { + 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, SetQuantifier::Any) => { - needle.clone().gt(array_min(haystack.clone())) - } - (BinaryOperator::Gt, SetQuantifier::All) => { + BinaryOperator::Gt => { Expr::Not(Box::new(needle.clone().gt(array_max(haystack.clone())))) } - (BinaryOperator::Lt, SetQuantifier::Any) => { - needle.clone().lt(array_max(haystack.clone())) - } - (BinaryOperator::Lt, SetQuantifier::All) => { + BinaryOperator::Lt => { Expr::Not(Box::new(needle.clone().lt(array_min(haystack.clone())))) } - (BinaryOperator::GtEq, SetQuantifier::Any) => { - needle.clone().gt_eq(array_min(haystack.clone())) - } - (BinaryOperator::GtEq, SetQuantifier::All) => { + BinaryOperator::GtEq => { Expr::Not(Box::new(needle.clone().gt_eq(array_max(haystack.clone())))) } - (BinaryOperator::LtEq, SetQuantifier::Any) => { - needle.clone().lt_eq(array_max(haystack.clone())) - } - (BinaryOperator::LtEq, SetQuantifier::All) => { + BinaryOperator::LtEq => { Expr::Not(Box::new(needle.clone().lt_eq(array_min(haystack.clone())))) } _ => { return plan_err!( - "Unsupported {quantifier}Op: '{compare_op}', only '=', '<>', '>', '<', '>=', '<=' are supported" + "Unsupported AllOp: '{compare_op}', only '=', '<>', '>', '<', '>=', '<=' are supported" ); } }; - let (vacuous_result, decisive_result) = match quantifier { - SetQuantifier::Any => (false, true), - SetQuantifier::All => (true, false), - }; - let null_bool = lit(ScalarValue::Boolean(None)); when(null_arr_check, null_bool.clone()) - .when(empty_check, lit(vacuous_result)) + .when(empty_check, lit(true)) .when(null_lhs_check, null_bool.clone()) - .when(decisive_condition, lit(decisive_result)) + .when(decisive_condition, lit(false)) .when(has_nulls, null_bool) - .otherwise(lit(vacuous_result)) + .otherwise(lit(true)) } #[cfg(test)] diff --git a/datafusion/sql/tests/cases/plan_to_sql.rs b/datafusion/sql/tests/cases/plan_to_sql.rs index 6c260dc019..62912c7ff8 100644 --- a/datafusion/sql/tests/cases/plan_to_sql.rs +++ b/datafusion/sql/tests/cases/plan_to_sql.rs @@ -370,7 +370,7 @@ fn roundtrip_statement_postgres_any_array_expr() -> Result<(), DataFusionError> sql: "select left from array where 1 = any(left);", parser_dialect: GenericDialect {}, unparser_dialect: UnparserPostgreSqlDialect {}, - expected: @r#"SELECT "array"."left" FROM "array" WHERE CASE WHEN "array"."left" IS NULL THEN NULL WHEN (cardinality("array"."left") = 0) THEN false WHEN 1 IS NULL THEN NULL WHEN 1 = ANY("array"."left") THEN true WHEN array_position("array"."left", NULL, 1) IS NOT NULL THEN NULL ELSE false END"#, + expected: @r#"SELECT "array"."left" FROM "array" WHERE 1 = ANY("array"."left")"#, ); Ok(()) } diff --git a/datafusion/sqllogictest/test_files/array/array_has.slt b/datafusion/sqllogictest/test_files/array/array_has.slt index abfd697a42..e343c1b1fa 100644 --- a/datafusion/sqllogictest/test_files/array/array_has.slt +++ b/datafusion/sqllogictest/test_files/array/array_has.slt @@ -517,18 +517,16 @@ logical_plan 03)----SubqueryAlias: test 04)------SubqueryAlias: t 05)--------Projection: -06)----------Filter: __common_expr_3 IS NULL AND Boolean(NULL) OR __common_expr_3 IN ([Utf8View("7f4b18de3cfeb9b4ac78c381ee2ad278"), Utf8View("a"), Utf8View("b"), Utf8View("c")]) IS NOT DISTINCT FROM Boolean(true) AND __common_expr_3 IS NOT NULL -07)------------Projection: substr(CAST(md5(CAST(generate_series().value AS Utf8View)) AS Utf8View), Int64(1), Int64(32)) AS __common_expr_3 -08)--------------TableScan: generate_series() projection=[value] +06)----------Filter: substr(CAST(md5(CAST(generate_series().value AS Utf8View)) AS Utf8View), Int64(1), Int64(32)) IN ([Utf8View("7f4b18de3cfeb9b4ac78c381ee2ad278"), Utf8View("a"), Utf8View("b"), Utf8View("c")]) +07)------------TableScan: generate_series() projection=[value] physical_plan 01)ProjectionExec: expr=[count(Int64(1))@0 as count(*)] 02)--AggregateExec: mode=Final, gby=[], aggr=[count(Int64(1))] 03)----CoalescePartitionsExec 04)------AggregateExec: mode=Partial, gby=[], aggr=[count(Int64(1))] -05)--------FilterExec: __common_expr_3@0 IS NULL AND NULL OR __common_expr_3@0 IN (SET) ([7f4b18de3cfeb9b4ac78c381ee2ad278, a, b, c]) IS NOT DISTINCT FROM true AND __common_expr_3@0 IS NOT NULL, projection=[] -06)----------ProjectionExec: expr=[substr(md5(CAST(value@0 AS Utf8View)), 1, 32) as __common_expr_3] -07)------------RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1 -08)--------------LazyMemoryExec: partitions=1, batch_generators=[generate_series: start=1, end=100000, batch_size=8192] +05)--------FilterExec: substr(md5(CAST(value@0 AS Utf8View)), 1, 32) IN (SET) ([7f4b18de3cfeb9b4ac78c381ee2ad278, a, b, c]), projection=[] +06)----------RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1 +07)------------LazyMemoryExec: partitions=1, batch_generators=[generate_series: start=1, end=100000, batch_size=8192] query I with test AS (SELECT substr(md5(i::text)::text, 1, 32) as needle FROM generate_series(1, 100000) t(i)) @@ -756,26 +754,26 @@ select 5 <= any(make_array()); false # Mixed NULL + non-NULL array where no non-NULL element satisfies the condition -# These return NULL because NULLs leave the result indeterminate +# These return false (NULLs are skipped by array_min/array_max) query B select 5 > any(make_array(6, NULL)); ---- -NULL +false query B select 5 < any(make_array(3, NULL)); ---- -NULL +false query B select 5 >= any(make_array(6, NULL)); ---- -NULL +false query B select 5 <= any(make_array(3, NULL)); ---- -NULL +false # Mixed NULL + non-NULL array where a non-NULL element satisfies the condition query B @@ -806,38 +804,33 @@ true query B select 5 <> any(make_array(5, NULL)); ---- -NULL +false -# All-NULL array: all operators should return NULL (unknown comparison) +# All-NULL array: all operators should return false query B select 5 > any(make_array(NULL::INT, NULL::INT)); ---- -NULL +false query B select 5 < any(make_array(NULL::INT, NULL::INT)); ---- -NULL +false query B select 5 >= any(make_array(NULL::INT, NULL::INT)); ---- -NULL +false query B select 5 <= any(make_array(NULL::INT, NULL::INT)); ---- -NULL +false query B select 5 <> any(make_array(NULL::INT, NULL::INT)); ---- -NULL - -query B -select 5 = any(make_array(NULL::INT, NULL::INT)); ----- -NULL +false # NULL left operand: should return NULL for non-empty arrays query B @@ -897,35 +890,6 @@ select 5 <> any(NULL::INT[]); ---- NULL -query B -select 5 = any(NULL::INT[]); ----- -NULL - -# NULL = ANY with non-empty array -query B -select NULL = any(make_array(1, 2, 3)); ----- -NULL - -# = ANY with no match, no NULLs -query B -select 5 = any(make_array(1, 2, 3)); ----- -false - -# = ANY with mixed NULL (satisfying) returns TRUE -query B -select 5 = any(make_array(5, NULL)); ----- -true - -# = ANY with mixed NULL (non-satisfying): NULLs leave result indeterminate -query B -select 5 = any(make_array(1, 2, NULL)); ----- -NULL - statement ok DROP TABLE any_op_test; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
