[I] TIMESTAMP WITH TIME ZONE can resolve to a timezone-naive type, and casting to it discards an existing timezone [datafusion]

["adriangb (via GitHub)" <gi...@apache.org>]

adriangb opened a new issue, #25166:
URL: https://github.com/apache/datafusion/issues/25166

   ### Describe the bug
   
   Two related type-level problems with `TIMESTAMP WITH TIME ZONE`. They share a root cause and I think they should be decided together.
   
   **1. `TIMESTAMP WITH TIME ZONE` can resolve to a timezone-*naive* Arrow type.**
   
   With the default configuration (`datafusion.execution.time_zone` unset):
   
   ```sql
   SELECT arrow_typeof('2024-01-01T12:00:00Z'::timestamptz) AS type,
          '2024-01-01T12:00:00Z'::timestamptz AS value;
   +---------------+---------------------+
   | type          | value               |
   +---------------+---------------------+
   | Timestamp(ns) | 2024-01-01T12:00:00 |
   +---------------+---------------------+
   ```
   
   The `Z` is accepted and then discarded. A SQL type whose name is "with time zone" produced an Arrow type with no time zone.
   
   **2. Casting an already timezone-aware value to `timestamptz` *overwrites* its timezone.**
   
   ```sql
   SET datafusion.execution.time_zone = 'UTC';
   SELECT arrow_typeof((t AT TIME ZONE 'Europe/Brussels')::timestamptz) FROM …;
   -- Timestamp(ns, "UTC")   -- the Brussels annotation is gone
   ```
   
   This is the root cause of #13962: `date_trunc('day', (t AT TIME ZONE 'Europe/Brussels')::timestamptz)` groups by UTC day while naming Brussels. Remove the `::timestamptz` and the same query is correct. It is also the same defect family as #23841 (`to_timestamp` discarding an aware input's timezone).
   
   ### To Reproduce
   
   Both snippets above, on DataFusion 55.0.0 (`da89c7c85b`).
   
   ### Expected behavior
   
   Two invariants, which I think are worth stating explicitly whatever the fix:
   
   1. **`TIMESTAMP WITH TIME ZONE` never resolves to `Timestamp(_, None)`.** When the session time zone is unset it should fall back to a fixed zone rather than to naive.
   2. **`CAST(x AS TIMESTAMPTZ)` where `x` is already aware preserves `x`'s own timezone.**
   
   ### Additional context
   
   **This is a traceable side effect, not a deliberate design.** The default changed from `"+00:00"` to `None` in #18359 (DataFusion 51.0.0). That change was deliberate and well reasoned — it followed #18017 making `now()`/`current_date()`/`current_time()` read the session timezone, and #18204 / #18081 proposed making the config an `Option`.
   
   But the whole of its effect on the SQL type system was this edit in `datafusion/sql/src/planner.rs`:
   
   ```diff
   -  Some(self.context_provider.options().execution.time_zone.clone())
   +  self.context_provider.options().execution.time_zone.clone()
   ```
   
   a mechanical unwrap because the field's type changed. The comment two lines above it is still in the tree today and still says:
   
   ```rust
   // OUTPUT: [ArrowDataType] Timestamp<TimeUnit, Some(Time Zone)>
   ```
   
   Neither the PR body, nor any of the three driver issues, nor the upgrade-guide note mentions `timestamptz` or `TIMESTAMP WITH TIME ZONE` — the note frames the change purely as *"to better support using the default timezone in scalar UDF functions such as `now`, `current_date`, `current_time`, and `to_timestamp`"*. The type-system consequence was never discussed.
   
   **PostgreSQL and DuckDB (measured, PG 17.11 / DuckDB 1.5.2)** both default `TimeZone` to the machine's zone, never unset, so `timestamptz` is always aware in both. Their type is also invariant to the session zone — only rendering changes:
   
   ```
   PG      SET TimeZone='UTC';              pg_typeof -> timestamp with time zone,  12:00:00+00
           SET TimeZone='Europe/Brussels';  pg_typeof -> timestamp with time zone,  13:00:00+01
   DuckDB  same in both
   ```
   
   So invariant 1 has two reference implementations. **Invariant 2 does not**, and this is worth being explicit about: neither system can represent a per-value timezone at all, so neither can arbitrate. Worse, copying PostgreSQL literally gives you the current behaviour — in PG, `tstz::timestamptz` renders in the *session* zone because the value never carried one. Invariant 2 has to be argued on DataFusion's own terms: Arrow puts the zone in the value, so an operation not asked to change it shouldn't. `AT TIME ZONE 'Z'` already means "convert to Z" and `arrow_cast` already means "give me exactly this Arrow type", so `::timestamptz` does not need to be a third way to change a timezone.
   
   **Note that simply changing the default would not fix #13962.** With the default set to UTC, invariant 1 holds but the cast still relabels Brussels to UTC and `date_trunc` still groups by the wrong day. Invariant 2 is the one that fixes it. Changing the default is a separate question, and would flip `now()`'s type a second time in five releases (aware ≤50 → naive 51+ → aware again), so it deserves its own discussion.
   
   Related: #13962 (user-visible symptom), #23841 (same defect in `to_timestamp`), #13212 (which timezone an implicit coercion should pick), #21116 (`TimestampWithOffset`, the direction the ecosystem is moving).
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org