adriangb opened a new pull request, #23729:
URL: https://github.com/apache/datafusion/pull/23729
## Which issue does this PR close?
- N/A. Small, self-contained enhancement to `unwrap_cast_in_comparison`;
happy to file a tracking issue if preferred.
## Rationale for this change
`unwrap_cast_in_comparison` could not unwrap a cast between the two date
types in a comparison, so a predicate like `CAST(date32_col AS Date64) <op>
date64_lit` (and the reverse `CAST(date64_col AS Date32) <op> date32_lit`)
never folded to a comparison on the bare column. Folding it lets the literal be
compared against the unmodified column, which keeps predicate pushdown /
pruning effective on date columns.
There were two coupled causes in `try_cast_literal_to_type`
(`datafusion/expr-common/src/casts.rs`):
1. `try_cast_numeric_literal` scaled both `Date32` and `Date64` literals by
the same target multiplier (`mul = 1`). But `Date32` counts **days** since the
epoch while `Date64` counts **milliseconds**, so a cross conversion needs a
factor of `MILLISECONDS_IN_DAY` (86_400_000), not 1. Because `mul = 1` was
wrong for this pair, the code could only have produced an incorrect value, so
today it safely declines rather than mis-folding: it is a latent limitation,
not a wrong-output bug.
2. `is_lossy_temporal_cast` classified every `Date <-> temporal` pair as
lossy, which swept in `Date32 <-> Date64` and blocked the unwrap outright.
## What changes are included in this PR?
Make `Date32 <-> Date64` fold with **exact-only** semantics, matching
`try_cast_literal_to_type`'s contract (returns `Some` only when the literal is
exactly representable in the target type):
- `Date32 -> Date64`: always exact, `days * MILLISECONDS_IN_DAY`
(overflow-guarded with checked arithmetic).
- `Date64 -> Date32`: exact **only** when `ms % MILLISECONDS_IN_DAY == 0` (a
whole-day boundary); otherwise returns `None` so the unwrap is skipped. This is
correct for every operator, including `=`, and mirrors the existing Decimal
scaling path in the same function (multiply when the target unit is coarser,
else divide only when evenly divisible).
- `is_lossy_temporal_cast` is relaxed so a date-to-date (and identity) cast
is no longer pre-classified as lossy; exactness is instead enforced by the
scaling returning `None` on an inexact value. `Date <-> Timestamp` remains
lossy as before.
Scope is intentionally limited to the `Date32 <-> Date64` scaling, the gate
relaxation needed for it, and tests.
## Are these changes tested?
Yes.
- `datafusion/expr-common/src/casts.rs`: unit tests pinning real numbers
(2025-01-01 = day 20089 = 1_735_689_600_000 ms), covering `Date32 -> Date64`
(exact), `Date64 -> Date32` on and off a whole-day boundary, the epoch,
negative/pre-epoch days, and same-type identity. `expect_cast` additionally
cross-checks each successful fold against the Arrow cast kernel. A test also
pins the relaxed `is_lossy_temporal_cast` behaviour for date pairs vs. `Date
<-> Timestamp`.
- `datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs`: end-to-end
simplifier tests that `CAST(date32_col AS Date64) = <whole-day date64 lit>`
simplifies to `date32_col = <date32 lit>` (also via `try_cast` and for `<`),
that `CAST(date64_col AS Date32) = <date32 lit>` simplifies to `date64_col =
<whole-day date64 lit>`, and that a non-whole-day literal does **not** simplify.
`cargo fmt --check` is clean and `cargo clippy -p datafusion-expr-common --
-D warnings` passes.
## Are there any user-facing changes?
No public API changes. The optimizer now additionally rewrites `Date32 <->
Date64` cast comparisons where it previously left them untouched; results are
unchanged, plans are simplified.
## Note for reviewers
The open PR apache/datafusion#23727 adds the `if from_type == to_type {
return false }` identity guard to this same function. This PR is based
independently on `main` and includes that identity line as part of the clean
gate shape here, so depending on merge order the two may need a trivial rebase
where those lines overlap.
--
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]
