vismaytiwari opened a new pull request, #23279: URL: https://github.com/apache/datafusion/pull/23279
## Which issue does this PR close? - Closes #22265. ## Rationale for this change `time + interval` (and `interval + time` / `time - interval`) returned an `Interval` instead of a `time`, so the value was never wrapped within the 24-hour clock: ``` > SELECT time '23:30' + interval '2 hours'; 25 hours 30 mins -- an Interval, not a time ``` PostgreSQL (and DuckDB) return a `time` that wraps around midnight: ``` 01:30:00 ``` The root cause is in type coercion: `time <op> interval` was coerced by widening the `time` operand into an `Interval`, so the addition happened between two intervals and the result kept the `Interval` type. ## What changes are included in this PR? Following the existing `Date - Date` special case (in the same two files), `time ± interval` is now handled explicitly: - **Coercion** (`expr-common/src/type_coercion/binary.rs`): `time + interval`, `interval + time`, and `time - interval` now coerce to `(time, interval)` with a `time` result type (the interval is normalized to `MonthDayNano`). `interval - time`, which is not meaningful, is left unchanged. - **Evaluation** (`physical-expr/src/expressions/binary.rs`): a new `apply_time_interval` adds/subtracts the interval's sub-day component and wraps the result modulo 24 hours. All four time units (`Time32(Second|Millisecond)`, `Time64(Microsecond|Nanosecond)`) and both operand orders are supported. Only the interval's `nanoseconds` affect a time-of-day; whole months and days are ignored, matching PostgreSQL (e.g. `time '10:00' + interval '1 day 2 hours'` = `12:00:00`). `time - time` (→ `Interval`) is unchanged. ## Are these changes tested? Yes. `datafusion/sqllogictest/test_files/datetime/arith_time_interval.slt` was previously a characterization test that documented the incorrect `Interval` output; it now asserts the correct wrapped `time` results — including wrapping past midnight in both directions (`22:00 + 3h → 01:00:00`, `02:00 - 3h → 23:00:00`) and ignoring whole days. ## Are there any user-facing changes? Yes — `time ± interval` now returns a `time` value (wrapped within 24 hours) instead of an `Interval`, aligning DataFusion with PostgreSQL and DuckDB. -- 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]
