adriangb commented on issue #13212:
URL: https://github.com/apache/datafusion/issues/13212#issuecomment-5587604780

   I compared this against PostgreSQL 17, DuckDB 1.5.2 and SQLite 3.51 locally 
to pin down what the correct behavior actually is.
   
   **PostgreSQL and DuckDB agree on every case, and they agree with the 
expectation in the issue description.** Both resolve `timestamptz - timestamp` 
by implicitly casting the naive side to `timestamptz` using the **session** 
timezone, and both return `interval`.
   
   Session timezone `Asia/Singapore` (= `+08:00` for DataFusion):
   
   | | PostgreSQL 17 | DuckDB 1.5.2 | DataFusion 54.0.0 |
   |---|---|---|---|
   | `'2024-11-01T00:00:00.0'::timestamptz` | `2024-11-01 00:00:00+08` | 
`2024-11-01 00:00:00+08` | `2024-11-01T00:00:00+08:00` ✅ |
   | `tstz - tstz` | `08:00:00` | `08:00:00` | `8 hours` ✅ |
   | **`tstz - ts`** | **`08:00:00`** | **`08:00:00`** | **`0`** ❌ |
   | `ts::timestamptz` | `2024-11-01 00:00:00+08` | `2024-11-01 00:00:00+08` | 
`2024-11-01T00:00:00+08:00` ✅ |
   | `tstz - ts` with tz = `UTC` | `00:00:00` | — | `0` ✅ |
   
   SQLite is not a useful reference here — it has no timezone-aware timestamp 
type and no session timezone at all. `CAST(x AS timestamptz)` falls through to 
NUMERIC affinity and returns `2024`.
   
   ### The scope is narrower than it looks
   
   Worth flagging for whoever picks this up: **the explicit cast in DataFusion 
already has the correct semantics today.** It shifts the instant, it doesn't 
just relabel the type:
   
   ```sql
   SET datafusion.execution.time_zone = '+08:00';
   CREATE TABLE t AS SELECT
     '2024-11-01T00:00:00.0'::timestamp        AS ts,
     '2024-11-01T00:00:00.0+00:00'::timestamptz AS tstz;
   
   SELECT to_unixtime(ts), to_unixtime(ts::timestamptz), to_unixtime(tstz) FROM 
t;
   +------------+---------------+------------+
   | 1730419200 | 1730390400    | 1730419200 |   -- ts::timestamptz correctly 
shifts -8h
   +------------+---------------+------------+
   
   SELECT tstz - ts, tstz - ts::timestamptz FROM t;
   +---------+---------+
   | 0 hours | 8 hours |   -- with the cast written by hand, the answer is 
already right
   +---------+---------+
   ```
   
   And `EXPLAIN VERBOSE` shows `type_coercion` leaves the expression completely 
untouched — no cast is inserted at all:
   
   ```
   | initial_logical_plan             | Projection: t.tstz - t.ts |
   | logical_plan after type_coercion | SAME TEXT AS ABOVE        |
   ```
   
   So this looks like it can be fixed purely in binary-operator type coercion, 
by inserting the cast that DataFusion already implements correctly — no 
arrow-rs change and no new optimizer rule needed. @Omega359's concern about 
table columns of `Timestamp(_, None)` holds though: the repro above uses a real 
column rather than a folded literal, and it reproduces there too, so the fix 
does need to live in coercion rather than in literal casting.
   
   ### One thing that will bite the fix
   
   Filed separately as https://github.com/apache/datafusion/issues/25084: with 
a **named** session timezone, that cast currently errors on DST boundaries.
   
   ```sql
   SET datafusion.execution.time_zone = 'America/New_York';
   SELECT '2024-11-03T01:30:00'::timestamp::timestamptz;
   -- Arrow error: Cast error: Cannot cast timezone to different timezone
   ```
   
   Unambiguous times are fine; it's specifically the ambiguous fall-back hour 
and the nonexistent spring-forward hour, because arrow-cast's 
`adjust_timestamp_to_timezone` uses `.single()` on the local-time resolution. 
PostgreSQL and DuckDB both resolve those deterministically and identically 
(ambiguous → later offset, gap → shifted forward). Once coercion starts 
inserting this cast automatically, any query mixing tz-naive and tz-aware 
timestamps under a named session timezone will start hitting it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to