fix: coerce timezone-naive minus timezone-aware timestamps at equal time units

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

@@ -285,6 +285,34 @@ impl<'a> BinaryTypeCoercer<'a> {
             return Ok(Signature { lhs, rhs, ret });
         }
         Plus | Minus | Multiply | Divide | Modulo  =>  {
+            // `Timestamp(u, Some(tz)) - Timestamp(u, None)` at *equal* units is
+            // the one mixed timezone-aware/naive pair arrow can subtract
+            // directly, so the `self.get_result` probe below would accept it
+            // and no cast would be inserted: arrow then subtracts the raw
+            // values, i.e. reads the naive operand as UTC. Every other unit
+            // pairing falls through to `temporal_coercion_strict_timezone`
+            // below and reads the naive operand in the aware operand's zone,
+            // as comparisons already do. Coerce here so that all unit pairings
+            // agree.
+            if self.op == &Minus
+                && matches!(
+                    (lhs, rhs),
+                    (Timestamp(_, Some(_)), Timestamp(_, None))
+                        | (Timestamp(_, None), Timestamp(_, Some(_)))
+                )
+                && let Some(coerced) = temporal_coercion_strict_timezone(lhs, rhs)
+            {
+                let ret = self.get_result(&coerced, &coerced).map_err(|e| {
+                    plan_datafusion_err!(
+                        "Cannot get result type for temporal operation {coerced} {} {coerced}: {e}", self.op
+                    )
+                })?;
+                return Ok(Signature {
+                    lhs: coerced.clone(),
+                    rhs: coerced,
+                    ret,
+                });
+            }
             if let Ok(ret) = self.get_result(lhs, rhs) {
 
                 // Temporal arithmetic, e.g. Date32 + Interval

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

@@ -475,3 +475,32 @@ fn test_decimal_precision_overflow_cross_variant() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn test_timestamp_minus_mixed_timezone_awareness() -> Result<()> {
+    let aware_tz: Arc<str> = Arc::from("America/New_York");
+    let aware_ms = DataType::Timestamp(Millisecond, Some(Arc::clone(&aware_tz)));
+    let aware_ns = DataType::Timestamp(Nanosecond, Some(Arc::clone(&aware_tz)));
+    let naive_ns = DataType::Timestamp(Nanosecond, None);
+
+    // Differing units already coerced to the aware zone; equal units did not.
+    // Both must now coerce both operands to the aware zone at the finer unit,
+    // in either operand order.
+    test_coercion_binary_rule!(aware_ms, naive_ns, Operator::Minus, aware_ns);
+    test_coercion_binary_rule!(naive_ns, aware_ms, Operator::Minus, aware_ns);
+    test_coercion_binary_rule!(aware_ns, naive_ns, Operator::Minus, aware_ns);
+    test_coercion_binary_rule!(naive_ns, aware_ns, Operator::Minus, aware_ns);
+
+    // Pairs that are both aware or both naive are untouched.
+    test_coercion_binary_rule!(aware_ns, aware_ns, Operator::Minus, aware_ns);
+    test_coercion_binary_rule!(naive_ns, naive_ns, Operator::Minus, naive_ns);
+    // Two different aware zones keep their own types (arrow subtracts the
+    // instants directly); unchanged by this rule.
+    let other_tz = DataType::Timestamp(Nanosecond, Some(Arc::from("+08:00")));
+    test_coercion_binary_rule!(aware_ns, other_tz, Operator::Minus, aware_ns, other_tz);
+
+    // For contrast, comparisons already coerced the mixed pair this way.
+    test_coercion_binary_rule!(aware_ms, naive_ns, Operator::Eq, aware_ns);
+
+    Ok(())
+}

datafusion/sqllogictest/test_files/datetime/timestamps.slt

@@ -2079,6 +2079,76 @@ SELECT '2000-01-01T00:00:00'::timestamp - '2010-01-01T00:00:00'::timestamp;
 ----
 -3653 days 0 hours 0 mins 0.000000000 secs
 
+# Subtracting a timezone-naive timestamp from a timezone-aware one reads the
+# naive operand in the aware operand's time zone at every time unit, as `=`
+# does. At equal units arrow can subtract the raw values directly, which used to
+# skip coercion and read the naive operand as UTC, so the answer depended on the
+# time unit of the aware operand.
+statement ok
+CREATE TABLE mixed_tz_minus AS SELECT
+  arrow_cast('2024-11-01T00:00:00-04:00', 'Timestamp(Nanosecond, Some("America/New_York"))') AS ts_tz_ns,
+  arrow_cast('2024-11-01T00:00:00-04:00', 'Timestamp(Millisecond, Some("America/New_York"))') AS ts_tz_ms,
+  '2024-11-01T00:00:00'::timestamp AS ts;
+
+# postgresql / duckdb (session time zone America/New_York): 00:00:00, 00:00:00, true
+query ??B
+SELECT ts_tz_ns - ts, ts_tz_ms - ts, ts_tz_ns = ts FROM mixed_tz_minus;
+----
+0 days 0 hours 0 mins 0.000000000 secs 0 days 0 hours 0 mins 0.000000000 secs true
+
+query ??
+SELECT ts - ts_tz_ns, ts - ts_tz_ms FROM mixed_tz_minus;
+----
+0 days 0 hours 0 mins 0.000000000 secs 0 days 0 hours 0 mins 0.000000000 secs
+
+# The same at every time unit, with the aware and naive operands at equal units:
+# seconds, milliseconds, microseconds and nanoseconds.
+statement ok
+CREATE TABLE mixed_tz_minus_units AS SELECT
+  arrow_cast('2024-11-01T00:00:00-04:00', 'Timestamp(Second, Some("America/New_York"))') AS ts_tz_s,
+  arrow_cast('2024-11-01T00:00:00-04:00', 'Timestamp(Millisecond, Some("America/New_York"))') AS ts_tz_ms,
+  arrow_cast('2024-11-01T00:00:00-04:00', 'Timestamp(Microsecond, Some("America/New_York"))') AS ts_tz_us,
+  arrow_cast('2024-11-01T00:00:00-04:00', 'Timestamp(Nanosecond, Some("America/New_York"))') AS ts_tz_ns,
+  arrow_cast('2024-11-01T00:00:00', 'Timestamp(Second, None)') AS ts_s,
+  arrow_cast('2024-11-01T00:00:00', 'Timestamp(Millisecond, None)') AS ts_ms,
+  arrow_cast('2024-11-01T00:00:00', 'Timestamp(Microsecond, None)') AS ts_us,
+  arrow_cast('2024-11-01T00:00:00', 'Timestamp(Nanosecond, None)') AS ts_ns;
+
+# postgresql / duckdb (session time zone America/New_York): 00:00:00 for every unit
+query ????
+SELECT ts_tz_s - ts_s, ts_tz_ms - ts_ms, ts_tz_us - ts_us, ts_tz_ns - ts_ns FROM mixed_tz_minus_units;
+----
+0 days 0 hours 0 mins 0 secs 0 days 0 hours 0 mins 0.000 secs 0 days 0 hours 0 mins 0.000000 secs 0 days 0 hours 0 mins 0.000000000 secs
+
+query ????
+SELECT ts_s - ts_tz_s, ts_ms - ts_tz_ms, ts_us - ts_tz_us, ts_ns - ts_tz_ns FROM mixed_tz_minus_units;
+----
+0 days 0 hours 0 mins 0 secs 0 days 0 hours 0 mins 0.000 secs 0 days 0 hours 0 mins 0.000000 secs 0 days 0 hours 0 mins 0.000000000 secs
+
+query BBBB
+SELECT ts_tz_s = ts_s, ts_tz_ms = ts_ms, ts_tz_us = ts_us, ts_tz_ns = ts_ns FROM mixed_tz_minus_units;
+----
+true true true true
+
+statement ok
+DROP TABLE mixed_tz_minus_units
+
+statement ok
+SET datafusion.explain.logical_plan_only = true
+
+query TT
+EXPLAIN SELECT ts_tz_ns - ts FROM mixed_tz_minus;
+----
+logical_plan
+01)Projection: mixed_tz_minus.ts_tz_ns - CAST(mixed_tz_minus.ts AS Timestamp(ns, "America/New_York"))
+02)--TableScan: mixed_tz_minus projection=[ts_tz_ns, ts]
+
+statement ok
+SET datafusion.explain.logical_plan_only = false
+
+statement ok
+DROP TABLE mixed_tz_minus
+
 # Interval - Timestamp => error
 query error Cannot coerce arithmetic expression Interval\(MonthDayNano\) - Timestamp\(ns\) to valid types
 SELECT i - ts1 from FOO;

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

@@ -122,6 +122,22 @@ Rewriting affected files with a current DataFusion version, for example using
 restores optimizations that depend on them. Preserve the required data ordering
 and ordering metadata when rewriting sorted files.
 
+### `Timestamp(u, Some(tz)) - Timestamp(u, None)` reads the naive operand in `tz`
+
+Subtracting a timezone-naive timestamp from a timezone-aware one _at the same
+time unit_ did not coerce at all in `55.0.0`: the naive operand was read as UTC,
+which differed both from `=` on the same pair of values and from every other
+pairing of time units. It now coerces like the rest, so the naive operand is
+read in the aware operand's time zone:
+
+```sql
+-- ts_tz is 2024-11-01T00:00:00-04:00 (America/New_York), ts is the naive
+-- 2024-11-01T00:00:00; ts_tz = ts is true in both versions
+-- 55.0.0: 0 days 4 hours 0 mins 0.000000000 secs
+-- 56.0.0: 0 days 0 hours 0 mins 0.000000000 secs
+SELECT ts_tz - ts FROM timestamps;
+```
+
 ### `ForeignSession::create_physical_plan` is unsupported
 
 `ForeignSession::create_physical_plan` no longer forwards to the library that