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: [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]
