You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/27 04:27:50 UTC
(datafusion) branch gh-readonly-queue/main/pr-25240-33574a1559e6d43da9072cd4aeeec25c519eab1a created (now b7ccbbd6f4)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-25240-33574a1559e6d43da9072cd4aeeec25c519eab1a
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at b7ccbbd6f4 fix: coerce timezone-naive minus timezone-aware timestamps at equal time units (#25240)
This branch includes the following new commits:
new b7ccbbd6f4 fix: coerce timezone-naive minus timezone-aware timestamps at equal time units (#25240)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: coerce timezone-naive minus timezone-aware timestamps at equal time units (#25240)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25240-33574a1559e6d43da9072cd4aeeec25c519eab1a
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit b7ccbbd6f41c012a599610c1ac386380ddab49b6
Author: Adrian Garcia Badaracco <17...@users.noreply.github.com>
AuthorDate: Sun Sep 27 04:27:22 2026 +0000
fix: coerce timezone-naive minus timezone-aware timestamps at equal time units (#25240)
## Which issue does this PR close?
- Part of https://github.com/apache/datafusion/issues/13212. It does not
close it: the session time zone is not consulted yet, this only makes
`-` agree with `=` and with itself.
## Rationale for this change
Subtracting a timezone-naive timestamp from a timezone-aware one gives a
different answer depending on the *time unit* of the aware operand, and
disagrees with `=` on the same pair of values. No session time zone is
involved. On `main`:
```sql
CREATE TABLE t 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;
SELECT ts_tz_ns - ts, ts_tz_ms - ts, ts_tz_ns = ts FROM t;
-- main: 4 hours | 0 hours | true
```
The two values compare equal yet are four hours apart, and the answer
flips with the storage unit. PostgreSQL 17 and DuckDB 1.5.2, with the
session time zone set to `America/New_York` (the only zone in play here,
since DataFusion has no session zone set), both return `00:00:00` and
`true`.
The cause is in `BinaryTypeCoercer::signature_inner`. For `Minus`, the
first thing the arithmetic arm does is ask arrow for a result type.
`Timestamp(u, Some(tz)) - Timestamp(u, None)` at *equal* units is the
one mixed pair arrow can subtract directly, so no cast is inserted and
arrow subtracts the raw values, reading the naive operand as UTC. Every
other unit pairing fails that probe and falls through to
`temporal_coercion_strict_timezone`, which casts the naive operand to
the aware operand's zone, which is also what `=` and the other
comparisons do.
## What changes are included in this PR?
For `Minus` on a mixed timezone-aware / timezone-naive pair, coerce
through `temporal_coercion_strict_timezone` before the arrow probe, so
every unit pairing inserts the same cast. The naive operand is then read
in the aware operand's time zone at every unit, as `=` already reads it.
Nothing else changes: pairs that are both aware or both naive, and pairs
of two different aware zones, are untouched.
## What is the testing strategy for this PR?
- `test_timestamp_minus_mixed_timezone_awareness` in
`datafusion/expr-common/src/type_coercion/binary/tests/arithmetic.rs`
pins the coerced types for both operand orders at equal and differing
units, and the untouched cases.
- An slt block in
`datafusion/sqllogictest/test_files/datetime/timestamps.slt` with the
reproduction above, both operand orders, and the plan showing the
inserted cast. Each expectation is the value PostgreSQL and DuckDB
return.
- The full sqllogictest suite passes; no other expectation changes.
## Are there any user-facing changes?
Yes, documented in the 56.0.0 upgrade guide: `Timestamp(u, Some(tz)) -
Timestamp(u, None)` at equal units now reads the naive operand in `tz`
rather than as UTC. With the values above, `ts_tz - ts` changes from 4
hours to 0 hours, matching `ts_tz = ts` being true.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5.1 <no...@anthropic.com>
---
datafusion/expr-common/src/type_coercion/binary.rs | 28 +++++++++
.../src/type_coercion/binary/tests/arithmetic.rs | 29 +++++++++
.../test_files/datetime/timestamps.slt | 70 ++++++++++++++++++++++
docs/source/library-user-guide/upgrading/56.0.0.md | 16 +++++
4 files changed, 143 insertions(+)
diff --git a/datafusion/expr-common/src/type_coercion/binary.rs b/datafusion/expr-common/src/type_coercion/binary.rs
index e7c20dde10..4ea03943a6 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 70a8fc0e35..5b5a8c9438 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<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(())
+}
diff --git a/datafusion/sqllogictest/test_files/datetime/timestamps.slt b/datafusion/sqllogictest/test_files/datetime/timestamps.slt
index 6894590e62..951c50e706 100644
--- a/datafusion/sqllogictest/test_files/datetime/timestamps.slt
+++ b/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;
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 fc5e9d2f03..d3b2d51e85 100644
--- a/docs/source/library-user-guide/upgrading/56.0.0.md
+++ b/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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org