nikolas-sapa opened a new pull request, #26742:
URL: https://github.com/apache/datafusion/pull/26742
## Which issue does this PR close?
- Closes #26671
## Rationale for this change
Comparing a `Duration` (e.g. `ts1 - ts2`) with an `INTERVAL` that has a days
or months
component silently returns wrong results. For example, both of these
returned `true`
for a 10-day duration:
```sql
select (timestamp '2026-01-11' - timestamp '2026-01-01') <= interval '3
days'; -- true (wrong)
select interval '3 days' >= (timestamp '2026-01-11' - timestamp
'2026-01-01'); -- true (wrong)
```
so range filters built on such comparisons silently filter the wrong rows.
Root cause: the mixed comparison coerced both sides to
`Interval(MonthDayNano)`.
arrow-cast represents the duration side as `IntervalMonthDayNano(0, 0,
nanos)`, and
`IntervalMonthDayNano`'s derived ordering compares months, then days, then
nanoseconds —
so any interval with non-zero days or months decides the comparison on a
field the
duration side can never influence.
## What changes are included in this PR?
- Mixed `Duration`/`Interval` comparisons now compare exact elapsed time:
both sides
coerce to `Duration(Nanosecond)` via a new
`duration_interval_comparison_coercion`,
routed from `comparison_coercion_inner` so it covers comparison operators,
`IN`
lists, `CASE`/`WHEN` and `BETWEEN`.
- Intervals whose months and days are zero (e.g. `INTERVAL '240 hours'`)
compare
exactly and correctly.
- Intervals with non-zero months or days now fail loudly when converted —
during
constant folding or at execution — with Arrow's "Cannot convert interval
containing
non-zero months or days to duration", instead of silently returning wrong
results.
`Interval(YearMonth)` and `Interval(DayTime)` have no exact duration
conversion and
fail at planning ("cannot infer common argument type"), as before.
- `Duration`-vs-`Duration` and `Interval`-vs-`Interval` comparison coercion
is unchanged:
both still compare via `Interval(MonthDayNano)`'s ordering, consistent
with Postgres
(so `interval '3 days' > interval '240 hours'` remains true). Only mixed
pairs change.
Design note for reviewers: an alternative semantics would treat a day as
exactly
86,400 seconds and only reject months-bearing intervals. This PR
deliberately follows
Arrow's cast semantics instead: a "day" added to a timezone-aware timestamp
is not
always 86,400 seconds (DST transitions), so treating days as fixed in
*comparisons*
would silently disagree with `ts + INTERVAL '1 day'` *arithmetic* — erroring
keeps the
two consistent, and mirrors Arrow's own refusal to convert. Happy to switch
to
`days == 0`-only-erroring if maintainers prefer.
## What is the testing strategy for this PR?
- Unit tests: `test_duration_interval_comparison_coercion` in
`datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs`
covers all six
comparison operators plus `IS DISTINCT FROM` / `IS NOT DISTINCT FROM`, all
four
duration units, both operand orders, the `YearMonth`/`DayTime` rejection,
and that
Duration-vs-Duration / Interval-vs-Interval coercions are unchanged.
- sqllogictest regressions in
`datafusion/sqllogictest/test_files/datetime/interval.slt`:
the constant-folded path (scalar duration vs scalar interval), the runtime
path
(column values, both operand orders), an `IN`-list with the mixed pair,
pure-nanosecond intervals comparing exactly, and the exact error messages
for
months/days-bearing intervals and for
`Interval(YearMonth)`/`Interval(DayTime)`.
- Verified locally: `cargo test -p datafusion-expr-common --lib` (222
passed),
the `datetime/interval` sqllogictest files, `cargo fmt --all -- --check`,
`cargo clippy --all-targets --workspace --all-features -- -D warnings`,
and the full
workspace suite via `RUST_BACKTRACE=1 cargo xtask ci step test workspace`
(13,043 passed / 0 failed / 15 ignored on the pre-amend tree; the amend
adds only
test assertions, re-verified by the targeted runs above).
## Are there any user-facing changes?
Yes — a behavior change from silently wrong results to explicit errors for
Duration-vs-Interval comparisons where the interval has non-zero months or
days;
pure-time intervals now compare correctly. No public API changes.
--
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]
