adriangb opened a new pull request, #24479:
URL: https://github.com/apache/datafusion/pull/24479
## Which issue does this PR close?
- Closes #23692.
## Rationale for this change
Combining two or more `IS NOT DISTINCT FROM` conditions with `AND` fails to
plan:
```sql
SELECT l.* FROM l LEFT ANTI JOIN r
ON l.a IS NOT DISTINCT FROM r.a AND l.b IS NOT DISTINCT FROM r.b
```
```
Error during planning: Cannot infer common argument type for logical boolean
operation Int64 AND Boolean
```
A single condition works, and adding parentheses around each condition
works, so the failure looks like a type coercion problem. It isn't. `sqlparser`
parses the right operand of `IS [NOT] DISTINCT FROM` with `parse_expr()` — that
is, at the lowest possible precedence — instead of stopping at the first
operator that binds less tightly than `IS`. Everything after the operator is
swallowed into its right operand:
```
l.a IS NOT DISTINCT FROM r.a AND l.b IS NOT DISTINCT FROM r.b
=> IsNotDistinctFrom(l.a, BinaryOp { r.a AND IsNotDistinctFrom(l.b, r.b) })
```
The `Int64 AND Boolean` in the error is that inner `r.a AND <bool>`.
PostgreSQL binds `AND` *less* tightly than `IS`, so the expected parse is `(l.a
IS NOT DISTINCT FROM r.a) AND (l.b IS NOT DISTINCT FROM r.b)`.
This affects every column type and every join type, and it also affects `IS
DISTINCT FROM` and plain `WHERE` clauses. It blocks multi-column equality
delete resolution in Apache Iceberg.
The same greedy `parse_expr()` call is still present on
`datafusion-sqlparser-rs` `main` (`src/parser/mod.rs:4074`), so the fix is
applied on the DataFusion side.
## What changes are included in this PR?
In `datafusion/sql/src/expr/mod.rs`, before planning an expression the
planner now restores the expected associativity:
1. `has_greedy_distinct_from` cheaply detects whether the mis-parse is
present. When it isn't — the overwhelmingly common case — nothing else runs.
2. `flatten_and_or` flattens the `AND` / `OR` spine into its first operand
plus the remaining `(operator, operand)` pairs, re-attaching each `IS [NOT]
DISTINCT FROM` to only the *first* operand of its right hand side.
3. `rebuild_and_or` rebuilds the expression with `AND` binding more tightly
than `OR`, both left associative.
Prefix `NOT` is handled the same way, since it also binds more tightly than
`AND` / `OR`. Handling `OR` and `NOT` is required for correctness rather than
completeness: a purely local rotation gets `a IS NOT DISTINCT FROM 1 AND b IS
NOT DISTINCT FROM 2 OR c IS NOT DISTINCT FROM 3` wrong, producing `A AND (B OR
C)` and turning a planning error into a silently wrong result.
Operands are not descended into, so a parenthesised sub-expression keeps its
explicit grouping and is handled when the planner recurses into it. The
rewrite's output is a fixed point — it never leaves an `IS [NOT] DISTINCT FROM`
whose right operand is an `AND` / `OR` — so re-entry cannot loop. Both
recursive helpers carry the crate's usual `recursive_protection` attribute.
### Known limitation left in place
The postfix `IS` family at the same precedence level (`a IS NOT DISTINCT
FROM b IS NULL`) is still associated as `sqlparser` produces it. That behaviour
is unchanged by this PR and belongs with the broader precedence discussion in
#22461.
## Are these changes tested?
Yes.
New planner tests in `datafusion/sql/tests/sql_integration.rs` covering join
`ON` clauses, `WHERE` clauses, projections, `IS DISTINCT FROM`, `AND` / `OR`
chains, and parenthesised forms as a control.
New end-to-end cases in
`datafusion/sqllogictest/test_files/join_is_not_distinct_from.slt`, including
the `LEFT ANTI JOIN` from the issue, three-condition chains, and the `AND` /
`OR` and `NOT` precedence cases. The precedence cases are chosen so that
incorrect grouping returns a different set of rows, not just a different plan —
the plan display alone does not distinguish `NOT (A AND B)` from `(NOT A) AND
B`.
Verification run:
- `cargo test -p datafusion-sql` — 565 + 87 + 12 doctests pass
- full sqllogictest suite — 501/501 files pass
- `cargo test -p datafusion-optimizer -p datafusion-expr` — passes
- `cargo clippy -p datafusion-sql -p datafusion-optimizer --all-targets --
-D warnings` — clean
- `cargo fmt --all` applied
## Are there any user-facing changes?
Yes, and they are the point of the PR: `IS [NOT] DISTINCT FROM` combined
with `AND` / `OR` / `NOT` without parentheses now parses the way PostgreSQL
parses it, so queries that previously failed to plan now succeed.
Queries that were already parenthesised are unaffected. No public API
changes.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FWoZ9timwHS7Az8nq3E7HS)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
