From 0577474cd68172b3d0c733d031f154036714619a Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:32:49 -0500 Subject: [PATCH 1/3] fix: coerce timezone-naive minus timezone-aware timestamps at equal units `Timestamp(u, Some(tz)) - Timestamp(u, None)` at *equal* units is the one mixed timezone-aware/naive pair that arrow can subtract directly, so `BinaryTypeCoercer` inserted no cast at all and arrow subtracted the raw values, reading the naive operand as UTC. Every other unit pairing falls through to `temporal_coercion_strict_timezone` and reads the naive operand in the aware operand's zone, which is what comparisons (`=`, `<`, ...) already do. Coerce the equal-unit case the same way so that all unit pairings agree. Co-Authored-By: Claude Fable 5.1 --- .../expr-common/src/type_coercion/binary.rs | 28 ++++++++++++++++++ .../type_coercion/binary/tests/arithmetic.rs | 29 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/datafusion/expr-common/src/type_coercion/binary.rs b/datafusion/expr-common/src/type_coercion/binary.rs index e7c20dde101b0..4ea03943a667c 100644 --- a/datafusion/expr-common/src/type_coercion/binary.rs +++ b/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 diff --git a/datafusion/expr-common/src/type_coercion/binary/tests/arithmetic.rs b/datafusion/expr-common/src/type_coercion/binary/tests/arithmetic.rs index 70a8fc0e35a15..5b5a8c94384d8 100644 --- a/datafusion/expr-common/src/type_coercion/binary/tests/arithmetic.rs +++ b/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 = 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(()) +} From f21731962f2f60fab16183e532b7d647b455afba Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Sat, 12 Sep 2026 13:55:50 -0500 Subject: [PATCH 2/3] test: pin mixed timezone-aware / naive subtraction at every time unit End to end reproduction of the unit-dependent answer: at nanoseconds the naive operand was read as UTC, at milliseconds in the aware operand's zone, so `ts_tz - ts` and `ts_tz = ts` disagreed. Both now match what PostgreSQL and DuckDB return with the aware operand's zone as the session time zone. Also documents the behaviour change in the 56.0.0 upgrade guide. Co-Authored-By: Claude Fable 5.1 --- .../test_files/datetime/timestamps.slt | 38 +++++++++++++++++++ .../library-user-guide/upgrading/56.0.0.md | 16 ++++++++ 2 files changed, 54 insertions(+) diff --git a/datafusion/sqllogictest/test_files/datetime/timestamps.slt b/datafusion/sqllogictest/test_files/datetime/timestamps.slt index fa4159c1dce23..6ddcdb8c28fdd 100644 --- a/datafusion/sqllogictest/test_files/datetime/timestamps.slt +++ b/datafusion/sqllogictest/test_files/datetime/timestamps.slt @@ -1980,6 +1980,44 @@ 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 + +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; diff --git a/docs/source/library-user-guide/upgrading/56.0.0.md b/docs/source/library-user-guide/upgrading/56.0.0.md index ef51e4c9a7ff6..68c093510e9ed 100644 --- a/docs/source/library-user-guide/upgrading/56.0.0.md +++ b/docs/source/library-user-guide/upgrading/56.0.0.md @@ -62,6 +62,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 From 472b554be193821d6d4fdd700339c2cd626bffcd Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Sat, 26 Sep 2026 22:35:28 -0500 Subject: [PATCH 3/3] test: cover mixed timezone subtraction at seconds and microseconds Address review: pin the equal-unit aware minus naive case at all four time units, in both operand orders, next to the equality it must agree with. Co-Authored-By: Claude Opus 5.5 --- .../test_files/datetime/timestamps.slt | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/datafusion/sqllogictest/test_files/datetime/timestamps.slt b/datafusion/sqllogictest/test_files/datetime/timestamps.slt index 6ddcdb8c28fdd..97d4b44713022 100644 --- a/datafusion/sqllogictest/test_files/datetime/timestamps.slt +++ b/datafusion/sqllogictest/test_files/datetime/timestamps.slt @@ -2002,6 +2002,38 @@ 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