Jefffrey commented on code in PR #23279:
URL: https://github.com/apache/datafusion/pull/23279#discussion_r3599656639
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -271,6 +271,183 @@ where
}
}
+/// Returns true for `time + interval` or `interval + time`.
+fn is_time_plus_interval(lhs: &DataType, rhs: &DataType) -> bool {
+ matches!(
+ (lhs, rhs),
+ (
+ DataType::Time32(_) | DataType::Time64(_),
+ DataType::Interval(_)
+ ) | (
+ DataType::Interval(_),
+ DataType::Time32(_) | DataType::Time64(_)
+ )
+ )
+}
+
+/// Returns true for `time - interval`.
+fn is_time_minus_interval(lhs: &DataType, rhs: &DataType) -> bool {
+ matches!(
+ (lhs, rhs),
+ (
+ DataType::Time32(_) | DataType::Time64(_),
+ DataType::Interval(_)
+ )
+ )
+}
+
+/// Evaluates `time + interval`, `interval + time`, or `time - interval`,
returning a
+/// `time` wrapped within the 24-hour clock to match PostgreSQL and DuckDB
(e.g.
+/// `time '23:30' + interval '2 hours'` is `01:30:00`). arrow's arithmetic
kernels do
+/// not implement time-of-day arithmetic, so it is handled here.
+///
+/// The result keeps the input time's unit; the interval (normalized to
`MonthDayNano`
+/// by the coercion layer) is applied at that resolution, mirroring `timestamp
+ interval`.
+/// Only the sub-day portion of the interval affects a time-of-day -- whole
months and
+/// days are ignored, matching PostgreSQL -- and any interval precision finer
than the
+/// time's unit is truncated (so `time(s) + interval '1 nanosecond'` is a
no-op).
Review Comment:
something to consider is how subtraction affects this. see (checked out on
this branch):
```sql
> select arrow_cast('2026-07-17T10:00:00', 'Timestamp(s)') - interval '1
nanosecond';
+----------------------------------------------------------------------------------------------------------------------------------------------------+
| arrow_cast(Utf8("2026-07-17T10:00:00"),Utf8("Timestamp(s)")) -
IntervalMonthDayNano("IntervalMonthDayNano { months: 0, days: 0, nanoseconds: 1
}") |
+----------------------------------------------------------------------------------------------------------------------------------------------------+
| 2026-07-17T09:59:59
|
+----------------------------------------------------------------------------------------------------------------------------------------------------+
1 row(s) fetched.
Elapsed 0.005 seconds.
> select arrow_cast('10:00:00', 'Time32(s)') - interval '1 nanosecond';
+--------------------------------------------------------------------------------------------------------------------------------------+
| arrow_cast(Utf8("10:00:00"),Utf8("Time32(s)")) -
IntervalMonthDayNano("IntervalMonthDayNano { months: 0, days: 0, nanoseconds: 1
}") |
+--------------------------------------------------------------------------------------------------------------------------------------+
| 10:00:00
|
+--------------------------------------------------------------------------------------------------------------------------------------+
1 row(s) fetched.
Elapsed 0.007 seconds.
```
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -267,6 +267,30 @@ impl<'a> BinaryTypeCoercer<'a> {
ret: Int64,
});
}
+ Plus | Minus if is_time_interval_arithmetic(lhs, rhs, self.op) => {
+ // `time ± interval` yields a `time` wrapped within the 24-hour
clock,
+ // matching PostgreSQL and DuckDB (e.g. `time '23:30' + interval
'2 hours'`
+ // is `01:30:00`). The interval is normalized to `MonthDayNano`;
the time
+ // operand keeps its own unit and is also the result type --
mirroring
+ // `timestamp/date + interval`, which preserve their unit and
apply the
+ // interval at that resolution. So, like `timestamp(s) + interval
+ // '1 nanosecond'`, `time(s) + interval '1 nanosecond'` is a no-op
rather
+ // than widening the type.
+ let time = if let Interval(_) = lhs {
+ rhs.clone()
+ } else {
+ lhs.clone()
+ };
+ let (lhs, rhs) = match (lhs, rhs) {
+ (Interval(_), _) => (Interval(MonthDayNano), time.clone()),
+ (_, _) => (time.clone(), Interval(MonthDayNano)),
+ };
+ return Ok(Signature {
+ lhs,
+ rhs,
+ ret: time,
+ });
Review Comment:
```suggestion
let (lhs, rhs, ret) = match (lhs, rhs) {
(Interval(_), time) => (Interval(MonthDayNano),
time.clone(), time.clone()),
(time, _) => (time.clone(), Interval(MonthDayNano),
time.clone()),
};
return Ok(Signature {
lhs,
rhs,
ret,
});
```
--
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]
