adriangb opened a new pull request, #23727:
URL: https://github.com/apache/datafusion/pull/23727
## Which issue does this PR close?
No existing issue; this PR both reports and fixes the bug. Happy to file a
tracking issue if preferred.
<!-- - Closes #. -->
## Rationale for this change
`unwrap_cast_in_comparison` fails to fold an identity `CAST(col AS DATE)` on
a `Date32` column. Instead of rewriting the predicate to compare against the
bare column, it leaves a residual `Cast(col AS Date32)` in place, which defeats
downstream optimizations (pruning / filter pushdown) that expect a bare-column
comparison.
Reproduction (logical plan for `SELECT * FROM t WHERE cast(d AS date) = DATE
'2024-01-01'`, where `d` is `Date32`): the `cast(d AS Date32)` survives
simplification rather than collapsing to `d`. Note `arrow_cast(d, 'Date32')`
already folds, because the `arrow_cast` UDF's `simplify()` short-circuits an
identity cast; the SQL `CAST ... AS date` planner plants a real `Expr::Cast`
with no such elision, so it reaches `try_cast_literal_to_type` and is wrongly
rejected.
The root cause is in `is_lossy_temporal_cast`
(`datafusion/expr-common/src/casts.rs`):
```rust
(is_date_type(from_type) && to_type.is_temporal())
|| (is_date_type(to_type) && from_type.is_temporal())
```
For an identity `Date32 -> Date32` cast this evaluates to `true && true`,
because `DataType::is_temporal()` is true for both `Date32` and `Date64`. The
identity cast is therefore misclassified as a lossy temporal cast,
`try_cast_literal_to_type` returns `None`, and `unwrap_cast_in_comparison`
leaves the cast in the plan.
## What changes are included in this PR?
Short-circuit an identity cast as non-lossy at the top of
`is_lossy_temporal_cast`:
```rust
if from_type == to_type {
return false; // an identity cast never changes comparison semantics
}
```
This is deliberately limited to *identical* types, not "any date-to-date".
`Date32` counts days while `Date64` counts milliseconds, but
`try_cast_numeric_literal` uses `mul = 1` for both, so allowing a `Date32 <->
Date64` unwrap would convert units incorrectly. The `from_type == to_type`
identity guard is the exact correct scope, and `Date32 <-> Date64` remains
blocked.
## Are these changes tested?
Yes:
- `test_try_cast_identity_date_allowed` (`expr-common`) — identity `Date32
-> Date32` / `Date64 -> Date64` now fold (`try_cast_literal_to_type` returns
`Some`), and `is_lossy_temporal_cast` reports them non-lossy.
- `test_try_cast_date32_date64_still_blocked` (`expr-common`) — `Date32 <->
Date64` remains lossy/blocked (guards the caveat above).
- `test_unwrap_identity_date_cast` (`optimizer`) — end-to-end:
`cast(date_col AS DATE) = DATE '...'` simplifies to `date_col = <lit>`.
All `datafusion-expr-common` and `datafusion-optimizer` tests pass; `cargo
fmt --check` and `cargo clippy -D warnings` are clean on the changed crate.
## Are there any user-facing changes?
No API changes. Queries with `CAST(date_col AS DATE)` predicates on `Date32`
columns simplify to bare-column comparisons, which can enable additional
pruning/pushdown. No behavioral change to query results.
--
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]
