fix: time ± interval returns a wrapped time instead of an interval

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

@@ -267,6 +267,22 @@ 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`). Both operands are normalized so the physical layer needs
+            // no casting: the time side to `Time64(Nanosecond)` and the interval side
+            // to `MonthDayNano`. The result is that wrapped `Time64(Nanosecond)`.
+            let (lhs, rhs) = match (lhs, rhs) {
+                (Interval(_), _) => (Interval(MonthDayNano), Time64(Nanosecond)),
+                (_, _) => (Time64(Nanosecond), Interval(MonthDayNano)),
+            };
+            return Ok(Signature {
+                lhs,
+                rhs,
+                ret: Time64(Nanosecond),
+            });
+        }
         Plus | Minus | Multiply | Divide | Modulo  =>  {
             if let Ok(ret) = self.get_result(lhs, rhs) {
 
@@ -362,6 +378,23 @@ fn is_date_minus_date(lhs: &DataType, rhs: &DataType) -> bool {
     )
 }
 
+/// Returns true for `time + interval`, `interval + time`, or `time - interval`.
+///
+/// These follow PostgreSQL/DuckDB semantics where the result is a `time` value
+/// wrapped within the 24-hour clock, rather than being widened to an interval.
+fn is_time_interval_arithmetic(lhs: &DataType, rhs: &DataType, op: &Operator) -> bool {
+    use DataType::{Interval, Time32, Time64};
+    match op {
+        Operator::Plus => matches!(
+            (lhs, rhs),
+            (Time32(_) | Time64(_), Interval(_)) | (Interval(_), Time32(_) | Time64(_))
+        ),
+        // `interval - time` is not meaningful, so only `time - interval` is accepted.
+        Operator::Minus => matches!((lhs, rhs), (Time32(_) | Time64(_), Interval(_))),
+        _ => false,
+    }
+}
+
 /// Coercion rules for mathematics operators between decimal and non-decimal types.
 fn math_decimal_coercion(
     lhs_type: &DataType,

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

@@ -274,6 +274,129 @@ 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 `Time64(Nanosecond)` 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 type coercion layer normalizes the operands to `Time64(Nanosecond)` and
+/// `Interval(MonthDayNano)`, so no casting is needed here. Only the sub-day portion
+/// of the interval (its `nanoseconds`) affects a time-of-day; whole months and days
+/// are ignored, matching PostgreSQL.
+fn apply_time_interval(
+    lhs: &ColumnarValue,
+    rhs: &ColumnarValue,
+    subtract: bool,
+) -> Result<ColumnarValue> {
+    /// Nanoseconds in a 24-hour day.
+    const DAY_NANOS: i64 = 86_400_000_000_000;
+
+    /// Wraps `time_ns ± interval.nanoseconds` into `[0, DAY_NANOS)`. The interval is
+    /// reduced modulo a day first so the addition stays within `i64`, and
+    /// `rem_euclid` keeps the result non-negative even when subtracting.
+    fn wrap(time_ns: i64, interval: IntervalMonthDayNano, subtract: bool) -> i64 {
+        let delta = interval.nanoseconds % DAY_NANOS;
+        let delta = if subtract { -delta } else { delta };
+        (time_ns + delta).rem_euclid(DAY_NANOS)
+    }
+
+    /// Extracts a `Time64(Nanosecond)` scalar as nanoseconds since midnight.
+    fn time_scalar_ns(scalar: &ScalarValue) -> Result<Option<i64>> {
+        match scalar {
+            ScalarValue::Time64Nanosecond(value) => Ok(*value),
+            other => {
+                internal_err!(
+                    "Time64(Nanosecond) scalar expected, got: {}",
+                    other.data_type()
+                )
+            }
+        }
+    }
+
+    /// Extracts an `Interval(MonthDayNano)` scalar.
+    fn interval_scalar(scalar: &ScalarValue) -> Result<Option<IntervalMonthDayNano>> {
+        match scalar {
+            ScalarValue::IntervalMonthDayNano(value) => Ok(*value),
+            other => {
+                internal_err!(
+                    "Interval(MonthDayNano) scalar expected, got: {}",
+                    other.data_type()
+                )
+            }
+        }
+    }
+
+    // The `time` operand determines the result; the other is the interval.
+    let (time, interval) = if matches!(lhs.data_type(), DataType::Interval(_)) {
+        (rhs, lhs)
+    } else {
+        (lhs, rhs)
+    };
+
+    match (time, interval) {
+        (ColumnarValue::Array(time), ColumnarValue::Array(interval)) => {
+            let time = time.as_primitive::<Time64NanosecondType>();
+            let interval = interval.as_primitive::<IntervalMonthDayNanoType>();
+            let result: Time64NanosecondArray =
+                arrow::compute::binary(time, interval, |t, iv| wrap(t, iv, subtract))?;
+            Ok(ColumnarValue::Array(Arc::new(result)))
+        }
+        (ColumnarValue::Array(time), ColumnarValue::Scalar(interval)) => {
+            let time = time.as_primitive::<Time64NanosecondType>();
+            match interval_scalar(interval)? {
+                Some(iv) => {
+                    let result: Time64NanosecondArray =
+                        time.unary(|t| wrap(t, iv, subtract));
+                    Ok(ColumnarValue::Array(Arc::new(result)))
+                }
+                None => Ok(ColumnarValue::Scalar(ScalarValue::Time64Nanosecond(None))),
+            }
+        }
+        (ColumnarValue::Scalar(time), ColumnarValue::Array(interval)) => {
+            let interval = interval.as_primitive::<IntervalMonthDayNanoType>();
+            match time_scalar_ns(time)? {
+                Some(t) => {
+                    let result: Time64NanosecondArray =
+                        interval.unary(|iv| wrap(t, iv, subtract));
+                    Ok(ColumnarValue::Array(Arc::new(result)))
+                }
+                None => Ok(ColumnarValue::Scalar(ScalarValue::Time64Nanosecond(None))),
+            }
+        }
+        (ColumnarValue::Scalar(time), ColumnarValue::Scalar(interval)) => {
+            let result = time_scalar_ns(time)?
+                .zip(interval_scalar(interval)?)
+                .map(|(t, iv)| wrap(t, iv, subtract));
+            Ok(ColumnarValue::Scalar(ScalarValue::Time64Nanosecond(result)))
+        }
+    }
+}
+
 impl PhysicalExpr for BinaryExpr {
     fn data_type(&self, input_schema: &Schema) -> Result<DataType> {
         BinaryTypeCoercer::new(
@@ -356,6 +479,18 @@ impl PhysicalExpr for BinaryExpr {
         let input_schema = schema.as_ref();
 
         match self.op {
+            // `time ± interval` returns a wrapped `time` (PostgreSQL/DuckDB
+            // semantics); arrow's arithmetic kernels don't implement it.
+            Operator::Plus
+                if is_time_plus_interval(&left_data_type, &right_data_type) =>
+            {
+                return apply_time_interval(&lhs, &rhs, false);
+            }
+            Operator::Minus
+                if is_time_minus_interval(&left_data_type, &right_data_type) =>
+            {
+                return apply_time_interval(&lhs, &rhs, true);
+            }
             Operator::Plus if self.fail_on_overflow => return apply(&lhs, &rhs, add),
             Operator::Plus => return apply(&lhs, &rhs, add_wrapping),
             // Special case: Date - Date returns Int64 (days difference)

datafusion/sqllogictest/test_files/datetime/arith_time_interval.slt

@@ -1,70 +1,102 @@
 # postgresql behavior
 #
 # time + interval → time
-# Add an interval to a time
+# Add an interval to a time. The result is a `time` value that wraps within the
+# 24-hour clock, matching PostgreSQL and DuckDB.
 # time '01:00' + interval '3 hours' → 04:00:00
-#
-# note that while the above reflects what postgresql does
-# in the case of datafusion/arrow that is not the case. The
-# result will be an interval, not a time.
+# time '22:00' + interval '3 hours' → 01:00:00 (wraps past midnight)
 
-query ?
+query D
 SELECT '01:00'::time + interval '3 hours'
 ----
-4 hours
+04:00:00
 
 query T
 SELECT arrow_typeof('01:00'::time + interval '3 hours')
 ----
-Interval(MonthDayNano)
+Time64(ns)
 
-query ?
+query D
 SELECT '22:00'::time + interval '3 hours'
 ----
-25 hours
+01:00:00
 
-query ?
+query D
 SELECT interval '3 hours' + '22:00'::time
 ----
-25 hours
+01:00:00
 
-query ?
+query D
 SELECT arrow_cast('22:00', 'Time32(Second)') + interval '3 hours'
 ----
-25 hours
+01:00:00
+
+# Regardless of the input time unit, the result is normalized to Time64(ns).
+query T
+SELECT arrow_typeof(arrow_cast('22:00', 'Time32(Second)') + interval '3 hours')
+----
+Time64(ns)
 
-query ?
+query D
 SELECT arrow_cast('22:00', 'Time32(Millisecond)') + interval '3 hours'
 ----
-25 hours
+01:00:00
 
-query ?
+query D
 SELECT arrow_cast('22:00', 'Time64(Microsecond)') + interval '3 hours'
 ----
-25 hours
+01:00:00
 
-query ?
+query D
 SELECT arrow_cast('22:00', 'Time64(Nanosecond)') + interval '3 hours'
 ----
-25 hours
+01:00:00
+
+# Whole days and months in the interval do not affect a time-of-day (PostgreSQL).
+query D
+SELECT '10:00'::time + interval '1 day 2 hours'
+----
+12:00:00
 
 # postgresql behavior
 #
 # time - interval → time
-# Subtract an interval from a time
+# Subtract an interval from a time, wrapping within the 24-hour clock.
 # time '05:00' - interval '2 hours' → 03:00:00
+# time '02:00' - interval '3 hours' → 23:00:00 (wraps before midnight)
 
-query ?
+query D
 SELECT '05:00'::time - interval '2 hours'
 ----
-3 hours
+03:00:00
 
 query T
 SELECT arrow_typeof('05:00'::time - interval '2 hours')
 ----
-Interval(MonthDayNano)
+Time64(ns)
 
-query ?
+query D
 SELECT '02:00'::time - interval '3 hours'
 ----
--1 hours
+23:00:00
+
+# Array inputs (not only scalars) exercise the columnar path, including nulls.
+statement ok
+CREATE TABLE time_vals(id INT, t TIME) AS VALUES (1, '01:00'::time), (2, '22:00'::time), (3, NULL);
+
+query D
+SELECT t + interval '3 hours' FROM time_vals ORDER BY id
+----
+04:00:00
+01:00:00
+NULL
+
+query D
+SELECT t - interval '2 hours' FROM time_vals ORDER BY id
+----
+23:00:00
+20:00:00
+NULL
+
+statement ok
+DROP TABLE time_vals

docs/source/library-user-guide/upgrading/55.0.0.md

@@ -460,3 +460,21 @@ type aliases:
 Implement the newly introduced types for your custom cache implementation.
 
 See [PR #22613](https://github.com/apache/datafusion/pull/22613) for details.
+
+### `time ± interval` now returns a `time` instead of an `interval`
+
+Adding or subtracting an `interval` to/from a `time` value now returns a `time`
+that wraps within the 24-hour clock, matching PostgreSQL and DuckDB. Previously
+DataFusion returned an `interval`.
+
+```sql
+-- 55.0.0 onwards: returns a Time64(Nanosecond)
+SELECT time '23:30:00' + interval '2 hours';
+-- 01:30:00
+```
+
+Only the sub-day portion of the interval affects the result; whole days and
+months are ignored, as in PostgreSQL. The result type is always
+`Time64(Nanosecond)`.
+
+See [PR #23279](https://github.com/apache/datafusion/pull/23279) for details.