You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "mbutrovich (via GitHub)" <gi...@apache.org> on 2026/10/23 17:14:56 UTC

[I] Comparing a `Duration` to an `Interval` with a days field returns wrong results (e.g. `ts - ts <= INTERVAL '3 days'`) [datafusion]

mbutrovich opened a new issue, #26671:
URL: https://github.com/apache/datafusion/issues/26671

   ### Describe the bug
   
   A timestamp difference compared to a day interval returns the wrong answer. The same happens for two intervals that mix `days` and `nanoseconds`. Found on `main` (`c76ed5396ad3f4a4dc9d9ceeddc419a146b871f9`, `datafusion-cli` 55.1.0) and on 55.0.0.
   
   [`temporal_coercion`](https://github.com/apache/datafusion/blob/c76ed5396ad3f4a4dc9d9ceeddc419a146b871f9/datafusion/expr-common/src/type_coercion/binary.rs#L2234-L2236) coerces any mix of `Interval` and `Duration` to `Interval(MonthDayNano)`. A `Duration(ns)` of 19 days becomes `(0 months, 0 days, 1.64e15 ns)`. arrow-rs orders `IntervalMonthDayNano` field by field, so that value sorts below `(0, 3, 0)`. The ordering is documented as intentional in [`IntervalMonthDayNano`](https://github.com/apache/arrow-rs/blob/df4dc3ede59bd43b5f71847231212381d068bb72/arrow-buffer/src/interval.rs#L60-L70), and the arrow-ord test asserts [that 30 days is not a month](https://github.com/apache/arrow-rs/blob/df4dc3ede59bd43b5f71847231212381d068bb72/arrow-ord/src/comparison.rs#L281). I did not trace the physical comparison past the coercion.
   
   The sqllogictests added with the `Duration` vs `Interval` coercion in #11876 only use second-sized intervals (`interval '1 seconds'`), which have `days = 0`, so they do not reach this case.
   
   This is related to #8468, which discusses whether intervals should be comparable at all. At the time that issue was filed, `interval '1 month' < interval '100 days'` returned an "Unsupported interval operator" error. On `main` the same kind of comparison now returns a value, and for mixed fields it is the wrong one.
   
   ### To Reproduce
   
   ```sql
   SELECT
     (TIMESTAMP '2024-01-20' - TIMESTAMP '2024-01-01') <= INTERVAL '3 days' AS le3,
     (TIMESTAMP '2024-01-20' - TIMESTAMP '2024-01-01') >  INTERVAL '3 days' AS gt3,
     INTERVAL '48 hours' <= INTERVAL '1 day' AS h48_le_d1,
     INTERVAL '30 days' < INTERVAL '1 month' AS d30_lt_m1,
     INTERVAL '25 hours' = INTERVAL '1 day 1 hour' AS h25_eq;
   ```
   
   ```
   +------+-------+-----------+-----------+--------+
   | le3  | gt3   | h48_le_d1 | d30_lt_m1 | h25_eq |
   +------+-------+-----------+-----------+--------+
   | true | false | true      | true      | false  |
   +------+-------+-----------+-----------+--------+
   ```
   
   Casting either side to `INTERVAL` does not change the result. The same predicate over a real table made 49,048 of 49,048 rows "within 3 days" when the maximum gap was 31.8 days.
   
   ### Expected behavior
   
   `le3` is `false` and `gt3` is `true`, because the difference is 19 days. DuckDB 2.0.0 returns `false` for `le3`, `false` for `h48_le_d1`, `false` for `d30_lt_m1` and `true` for `h25_eq` (I did not run `gt3`).
   
   PostgreSQL combines the fields before comparing. [`interval_cmp_value`](https://github.com/postgres/postgres/blob/126976a8ffcfa47f40d3df09243ebf4606001912/src/backend/utils/adt/timestamp.c#L2542-L2561) turns months into 30 days and days into 24 hours. I read that function and did not run PostgreSQL. The outputs in #8468 show the same 30-day month.
   
   Should DataFusion normalize before comparing, or reject mixed comparisons with an error, as it did for `Duration` vs `Interval` before #11876? Normalizing in DataFusion looks smaller than changing arrow-rs, whose ordering is a documented contract.
   
   ### Additional context
   
   Rewrites that avoid the problem today are `a <= b + INTERVAL '3 days'`, `(a - b) <= INTERVAL '72 hours'`, and `EXTRACT(EPOCH FROM (a - b)) <= 259200`.
   
   I found this reproducing wrong results reported by the DB Olympics benchmark, tracked in #26661, where five queries use `ts - ts <= INTERVAL 'n days'`. Rewriting them to the addition form makes their results match the reference results.
   
   ### Willingness to contribute
   
   I cannot contribute a fix for this bug at this time
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] [dbolympics] Comparing a `Duration` to an `Interval` with a days field returns wrong results (e.g. `ts - ts <= INTERVAL '3 days'`) [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #26671:
URL: https://github.com/apache/datafusion/issues/26671#issuecomment-6425099659

   > Hi — I dug into this and opened PR https://github.com/apache/datafusion/pull/26742 with a fix. Thanks for the clear repro!
   
   Can you please distill down your question into something that is easier to understand? for exmaple I don't know off hand what arrow-array types.rs:1095-1105 contains.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] [dbolympics] Comparing a `Duration` to an `Interval` with a days field returns wrong results (e.g. `ts - ts <= INTERVAL '3 days'`) [datafusion]

Posted by "nikolas-sapa (via GitHub)" <gi...@apache.org>.
nikolas-sapa commented on issue #26671:
URL: https://github.com/apache/datafusion/issues/26671#issuecomment-6424993031

   Hi — I dug into this and opened PR #26742 with a fix. Thanks for the clear repro!
   
   **Root cause.** Comparison coercion unifies any `Duration` with any `Interval` as `Interval(MonthDayNano)`, and arrow's duration→interval cast puts the *entire* duration into the nanoseconds field (`IntervalMonthDayNano::new(0, 0, v)` in arrow-cast 60.0.0 `cast/mod.rs:516-568`). But `IntervalMonthDayNano`'s derived `Ord` is lexicographic **months → days → nanoseconds** (arrow-buffer `interval.rs:46-77` — its docs warn the ordering is "potentially surprising"). So for `(ts1 - ts2) <= INTERVAL '3 days'`, the duration becomes `{months: 0, days: 0, nanos: 10d}` and the interval `{months: 0, days: 3, nanos: 0}` — the days field decides `0 < 3` before nanoseconds are ever compared, and the predicate folds to `true`. Any interval with a non-zero months or days field mis-decides the same way; pure-nanosecond intervals (`INTERVAL '1 hour'`) happened to compare correctly because the comparison degenerated to a pure nanoseconds compare. Both the constant-folded and the runtime path
  are affected.
   
   **Proposed semantics** (in the PR): mixed Duration-vs-Interval comparisons coerce both sides to `Duration(Nanosecond)` and compare exact elapsed time, following arrow's own convertibility rules:
   
   - arrow-cast only supports `Duration <-> Interval(MonthDayNano)` casts at all (`cast/mod.rs:336-337`), so `Interval(YearMonth)`/`Interval(DayTime)` mixed comparisons are rejected at planning (`Cannot infer common argument type for comparison operation`).
   - `cast_month_day_nano_to_duration` succeeds exactly when `months == 0 && days == 0` and otherwise fails loudly (`Cannot convert interval containing non-zero months or days to duration`). With DataFusion's default `safe: false` cast options, pure-nanosecond intervals keep comparing correctly via exact nanoseconds, while days/months intervals error — at plan time for literals, at execution time for columns — instead of silently returning wrong results.
   - Duration-vs-Duration, Interval-vs-Interval, and UNION/CASE type unification are unchanged.
   
   **One judgment call I'd appreciate maintainer feedback on:** for intervals with a **days** component, an alternative would be to normalize days as exact 24h (`days * 86_400e9 + nanos`) and reject only months (arrow-arith does add interval days as exact 24h, `arrow-array` `types.rs:1095-1105`). I went with erroring instead, because arrow-cast deliberately refuses interval→duration for non-zero days, and hand-rolling the conversion would make `d <= i` disagree with `CAST(i AS Duration(...))` / `arrow_cast(i, 'Duration(Second)')`. If you'd rather keep `d <= INTERVAL '3 days'` working via exact-24h normalization, that's a small change to the coercion and I'm happy to do it.
   
   (Prepared with AI assistance; analysis and fix verified locally — unit tests plus sqllogictests for both the constant-folded and runtime paths, all green.)
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] [dbolympics] Comparing a `Duration` to an `Interval` with a days field returns wrong results (e.g. `ts - ts <= INTERVAL '3 days'`) [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #26671:
URL: https://github.com/apache/datafusion/issues/26671#issuecomment-6384964373

   Affects DB Olympics, see: https://github.com/apache/datafusion/issues/26661


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org