Skip to content

Commit 0a33c81

Browse files
committed
fix: time ± interval returns a wrapped time instead of an interval
`time + interval`, `interval + time`, and `time - interval` previously widened the time operand into an interval, so `time '23:30' + interval '2 hours'` returned a `25 hours 30 mins` interval rather than wrapping within the 24-hour clock. Following the existing `Date - Date` special case, coercion now keeps the result as `time`, and a new `apply_time_interval` kernel adds/subtracts the interval's sub-day component modulo 24 hours to match PostgreSQL and DuckDB. Whole months and days are ignored, and all four time units are supported. Closes #22265
1 parent 9e8dd76 commit 0a33c81

3 files changed

Lines changed: 143 additions & 26 deletions

File tree

datafusion/expr-common/src/type_coercion/binary.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,21 @@ impl<'a> BinaryTypeCoercer<'a> {
267267
ret: Int64,
268268
});
269269
}
270+
Plus | Minus if is_time_interval_arithmetic(lhs, rhs, self.op) => {
271+
// `time ± interval` yields a `time` wrapped within the 24-hour clock,
272+
// matching PostgreSQL and DuckDB (e.g. `time '23:30' + interval '2 hours'`
273+
// is `01:30:00`). The interval is normalized to `MonthDayNano` so the
274+
// physical layer only has to handle a single representation.
275+
let (lhs, rhs, ret) = match (lhs, rhs) {
276+
(Interval(_), time_type) => {
277+
(Interval(MonthDayNano), time_type.clone(), time_type.clone())
278+
}
279+
(time_type, _) => {
280+
(time_type.clone(), Interval(MonthDayNano), time_type.clone())
281+
}
282+
};
283+
return Ok(Signature { lhs, rhs, ret });
284+
}
270285
Plus | Minus | Multiply | Divide | Modulo => {
271286
if let Ok(ret) = self.get_result(lhs, rhs) {
272287

@@ -362,6 +377,23 @@ fn is_date_minus_date(lhs: &DataType, rhs: &DataType) -> bool {
362377
)
363378
}
364379

380+
/// Returns true for `time + interval`, `interval + time`, or `time - interval`.
381+
///
382+
/// These follow PostgreSQL/DuckDB semantics where the result is a `time` value
383+
/// wrapped within the 24-hour clock, rather than being widened to an interval.
384+
fn is_time_interval_arithmetic(lhs: &DataType, rhs: &DataType, op: &Operator) -> bool {
385+
use DataType::{Interval, Time32, Time64};
386+
match op {
387+
Operator::Plus => matches!(
388+
(lhs, rhs),
389+
(Time32(_) | Time64(_), Interval(_)) | (Interval(_), Time32(_) | Time64(_))
390+
),
391+
// `interval - time` is not meaningful, so only `time - interval` is accepted.
392+
Operator::Minus => matches!((lhs, rhs), (Time32(_) | Time64(_), Interval(_))),
393+
_ => false,
394+
}
395+
}
396+
365397
/// Coercion rules for mathematics operators between decimal and non-decimal types.
366398
fn math_decimal_coercion(
367399
lhs_type: &DataType,

datafusion/physical-expr/src/expressions/binary.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,74 @@ where
274274
}
275275
}
276276

277+
/// Returns true for `time + interval` or `interval + time`.
278+
fn is_time_plus_interval(lhs: &DataType, rhs: &DataType) -> bool {
279+
matches!(
280+
(lhs, rhs),
281+
(DataType::Time32(_) | DataType::Time64(_), DataType::Interval(_))
282+
| (DataType::Interval(_), DataType::Time32(_) | DataType::Time64(_))
283+
)
284+
}
285+
286+
/// Returns true for `time - interval`.
287+
fn is_time_minus_interval(lhs: &DataType, rhs: &DataType) -> bool {
288+
matches!(
289+
(lhs, rhs),
290+
(DataType::Time32(_) | DataType::Time64(_), DataType::Interval(_))
291+
)
292+
}
293+
294+
/// Evaluates `time + interval`, `interval + time`, or `time - interval`, returning
295+
/// a `time` wrapped within the 24-hour clock to match PostgreSQL and DuckDB
296+
/// (e.g. `time '23:30' + interval '2 hours'` is `01:30:00`). arrow's arithmetic
297+
/// kernels do not implement time-of-day arithmetic, so it is handled here.
298+
///
299+
/// Only the sub-day portion of the interval (its `nanoseconds`) affects a
300+
/// time-of-day; whole months and days are ignored, matching PostgreSQL.
301+
fn apply_time_interval(
302+
lhs: &ColumnarValue,
303+
rhs: &ColumnarValue,
304+
subtract: bool,
305+
num_rows: usize,
306+
) -> Result<ColumnarValue> {
307+
/// Nanoseconds in a 24-hour day.
308+
const DAY_NANOS: i128 = 86_400_000_000_000;
309+
310+
let left = lhs.to_array(num_rows)?;
311+
let right = rhs.to_array(num_rows)?;
312+
313+
// The `time` operand determines the result type; the other is the interval.
314+
let left_is_time =
315+
matches!(left.data_type(), DataType::Time32(_) | DataType::Time64(_));
316+
let (time_array, interval_array) =
317+
if left_is_time { (&left, &right) } else { (&right, &left) };
318+
let time_type = time_array.data_type().clone();
319+
320+
// Normalize to a single representation: time as Time64(ns), interval as MonthDayNano.
321+
let time_ns_arr = cast(time_array, &DataType::Time64(TimeUnit::Nanosecond))?;
322+
let time_ns = time_ns_arr.as_primitive::<Time64NanosecondType>();
323+
let interval_arr =
324+
cast(interval_array, &DataType::Interval(IntervalUnit::MonthDayNano))?;
325+
let interval = interval_arr.as_primitive::<IntervalMonthDayNanoType>();
326+
327+
let wrapped: Time64NanosecondArray =
328+
arrow::compute::binary(time_ns, interval, |t, iv| {
329+
let delta = iv.nanoseconds as i128;
330+
let total = if subtract {
331+
t as i128 - delta
332+
} else {
333+
t as i128 + delta
334+
};
335+
// Rust's `%` keeps the sign of the dividend, so add a day before the
336+
// final modulo to always land in `[0, DAY_NANOS)`.
337+
(((total % DAY_NANOS) + DAY_NANOS) % DAY_NANOS) as i64
338+
})?;
339+
340+
// Restore the original time unit (e.g. Time32(Second)).
341+
let result = cast(&(Arc::new(wrapped) as ArrayRef), &time_type)?;
342+
Ok(ColumnarValue::Array(result))
343+
}
344+
277345
impl PhysicalExpr for BinaryExpr {
278346
fn data_type(&self, input_schema: &Schema) -> Result<DataType> {
279347
BinaryTypeCoercer::new(
@@ -356,6 +424,18 @@ impl PhysicalExpr for BinaryExpr {
356424
let input_schema = schema.as_ref();
357425

358426
match self.op {
427+
// `time ± interval` returns a wrapped `time` (PostgreSQL/DuckDB
428+
// semantics); arrow's arithmetic kernels don't implement it.
429+
Operator::Plus
430+
if is_time_plus_interval(&left_data_type, &right_data_type) =>
431+
{
432+
return apply_time_interval(&lhs, &rhs, false, batch.num_rows());
433+
}
434+
Operator::Minus
435+
if is_time_minus_interval(&left_data_type, &right_data_type) =>
436+
{
437+
return apply_time_interval(&lhs, &rhs, true, batch.num_rows());
438+
}
359439
Operator::Plus if self.fail_on_overflow => return apply(&lhs, &rhs, add),
360440
Operator::Plus => return apply(&lhs, &rhs, add_wrapping),
361441
// Special case: Date - Date returns Int64 (days difference)
Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,70 +1,75 @@
11
# postgresql behavior
22
#
33
# time + interval → time
4-
# Add an interval to a time
4+
# Add an interval to a time. The result is a `time` value that wraps within the
5+
# 24-hour clock, matching PostgreSQL and DuckDB.
56
# time '01:00' + interval '3 hours' → 04:00:00
6-
#
7-
# note that while the above reflects what postgresql does
8-
# in the case of datafusion/arrow that is not the case. The
9-
# result will be an interval, not a time.
7+
# time '22:00' + interval '3 hours' → 01:00:00 (wraps past midnight)
108

11-
query ?
9+
query D
1210
SELECT '01:00'::time + interval '3 hours'
1311
----
14-
4 hours
12+
04:00:00
1513

1614
query T
1715
SELECT arrow_typeof('01:00'::time + interval '3 hours')
1816
----
19-
Interval(MonthDayNano)
17+
Time64(ns)
2018

21-
query ?
19+
query D
2220
SELECT '22:00'::time + interval '3 hours'
2321
----
24-
25 hours
22+
01:00:00
2523

26-
query ?
24+
query D
2725
SELECT interval '3 hours' + '22:00'::time
2826
----
29-
25 hours
27+
01:00:00
3028

31-
query ?
29+
query D
3230
SELECT arrow_cast('22:00', 'Time32(Second)') + interval '3 hours'
3331
----
34-
25 hours
32+
01:00:00
3533

36-
query ?
34+
query D
3735
SELECT arrow_cast('22:00', 'Time32(Millisecond)') + interval '3 hours'
3836
----
39-
25 hours
37+
01:00:00
4038

41-
query ?
39+
query D
4240
SELECT arrow_cast('22:00', 'Time64(Microsecond)') + interval '3 hours'
4341
----
44-
25 hours
42+
01:00:00
4543

46-
query ?
44+
query D
4745
SELECT arrow_cast('22:00', 'Time64(Nanosecond)') + interval '3 hours'
4846
----
49-
25 hours
47+
01:00:00
48+
49+
# Whole days and months in the interval do not affect a time-of-day (PostgreSQL).
50+
query D
51+
SELECT '10:00'::time + interval '1 day 2 hours'
52+
----
53+
12:00:00
5054

5155
# postgresql behavior
5256
#
5357
# time - interval → time
54-
# Subtract an interval from a time
58+
# Subtract an interval from a time, wrapping within the 24-hour clock.
5559
# time '05:00' - interval '2 hours' → 03:00:00
60+
# time '02:00' - interval '3 hours' → 23:00:00 (wraps before midnight)
5661

57-
query ?
62+
query D
5863
SELECT '05:00'::time - interval '2 hours'
5964
----
60-
3 hours
65+
03:00:00
6166

6267
query T
6368
SELECT arrow_typeof('05:00'::time - interval '2 hours')
6469
----
65-
Interval(MonthDayNano)
70+
Time64(ns)
6671

67-
query ?
72+
query D
6873
SELECT '02:00'::time - interval '3 hours'
6974
----
70-
-1 hours
75+
23:00:00

0 commit comments

Comments
 (0)