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

[adriangb]

Which issue does this PR close?

• Part of datafusion.execution.time_zone is not used for basic time zone inference #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:

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

[codecov-commenter]

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.50%. Comparing base (33574a1) to head (cb60a7f).

Files with missing lines	Patch %	Lines
datafusion/expr-common/src/type_coercion/binary.rs	76.92%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25240      +/-   ##
==========================================
- Coverage   82.50%   82.50%   -0.01%     
==========================================
  Files        1141     1141              
  Lines      438833   438846      +13     
  Branches   438833   438846      +13     
==========================================
+ Hits       362056   362062       +6     
- Misses      54895    54897       +2     
- Partials    21882    21887       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kosiew]

@adriangb,

Thanks for fixing the inconsistent timestamp subtraction. I have one non-blocking suggestion inline.

[kosiew]

This SQL test covers nanoseconds and milliseconds, but the comment says the fix applies at every time unit. Could you add aware timestamps in seconds and microseconds, with subtraction checks, to cover all four units?

[adriangb]

Thanks, 472b554