sesteves opened a new issue, #24096:
URL: https://github.com/apache/datafusion/issues/24096
### Describe the bug
The physical `BinaryExpr::nullable` implementation reports `IS DISTINCT
FROM` and `IS NOT DISTINCT FROM` as nullable when either operand is nullable:
```rust
fn nullable(&self, input_schema: &Schema) -> Result<bool> {
Ok(self.left.nullable(input_schema)? ||
self.right.nullable(input_schema)?)
}
```
SQL distinctness operators always produce a non-null Boolean, including when
one or both operands are null. The logical expressions correctly report
non-nullable output, but physical planning lowers Boolean truth predicates such
as `b IS TRUE` through `IsNotDistinctFrom`. A nullable input therefore creates
a logical/physical schema mismatch.
This can fail deterministically during aggregate physical planning when
common-subexpression elimination extracts a repeated nullable truth predicate:
```text
Physical input schema should be the same as the one converted from logical
input schema.
Differences: field nullability at index 0 [__common_expr_1]:
(physical) true vs (logical) false
```
I confirmed the generic nullable implementation is still present on
DataFusion `main` at commit `2dd1a14232b8021cb8979b0438367f1fde7c97e7`. The
behavior is also present in DataFusion 53.1.0 and 54.0.0.
### To Reproduce
The following query repeats a truth predicate over a nullable Boolean,
causing aggregate CSE to extract it:
```sql
SELECT
SUM(CASE WHEN b IS TRUE THEN 1 ELSE 0 END),
COUNT(CASE WHEN b IS TRUE THEN 1 END),
SUM(CASE WHEN b IS FALSE THEN 1 ELSE 0 END),
SUM(CASE WHEN b IS NOT TRUE THEN 1 ELSE 0 END),
SUM(CASE WHEN b IS NOT FALSE THEN 1 ELSE 0 END)
FROM (
VALUES
(TRUE),
(FALSE),
(CAST(NULL AS BOOLEAN))
) AS t(b);
```
The expected result is:
```text
1, 1, 1, 2, 2
```
A lower-level regression can construct physical `BinaryExpr` values using
nullable operands and verify that both `Operator::IsDistinctFrom` and
`Operator::IsNotDistinctFrom` report `nullable() == false`.
### Expected behavior
Physical distinctness expressions should always report non-nullable output:
```rust
match self.op {
Operator::IsDistinctFrom | Operator::IsNotDistinctFrom => Ok(false),
_ => Ok(self.left.nullable(input_schema)? ||
self.right.nullable(input_schema)?),
}
```
The repeated aggregate query should plan and execute while aggregate
logical/physical schema validation remains enabled.
Regression coverage should include:
- Nullable operands for both physical distinctness operators.
- Repeated nullable `IS TRUE` conditions that trigger aggregate CSE.
- `IS TRUE`, `IS FALSE`, `IS NOT TRUE`, and `IS NOT FALSE` semantics for
`TRUE`, `FALSE`, and `NULL` inputs.
### Additional context
This is related to the broad class of logical/physical schema nullability
mismatches discussed in #17801, but that issue concerned CASE/COALESCE
inference and does not fix the physical `BinaryExpr::nullable` path.
It is also distinct from #23692, which concerns SQL parser precedence for
multiple `IS NOT DISTINCT FROM` predicates in join conditions.
A downstream workaround can rewrite truth predicates to non-nullable CASE
expressions before physical planning, but the underlying physical distinctness
nullability should be fixed in DataFusion so downstream engines do not need
optimizer compatibility rules.
--
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]