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: [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]

Reply via email to