`to_timestamp_*` discards the timezone of already-timezone-aware inputs
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Describe the bug
Since #19078, the `to_timestamp` / `to_timestamp_seconds` / `to_timestamp_millis` / `to_timestamp_micros` / `to_timestamp_nanos` functions override the timezone of an input that is already timezone-aware with the session's `datafusion.execution.time_zone`.
When that config is unset (the default `None`), an input of type `Timestamp(_, Some(tz))` is returned as `Timestamp(_, None)`. The zone annotation is dropped while the underlying epoch is kept, which silently changes the wall-clock meaning of the value.
Previously, these functions had a dedicated match arm that preserved a zoned input's timezone:
```rust
// before #19078
Timestamp(_, Some(tz)) => args[0].cast_to(&Timestamp(Millisecond, Some(tz)), None),
```
#19078 collapsed that arm into the generic one, which casts to the execution timezone:
```rust
// after #19078 (current main)
Null | Int32 | Int64 | Timestamp(_, _) => args[0].cast_to(&Timestamp(Millisecond, self.timezone.clone()), None),
```
The PR's rationale when I read it, only concerns naive timestamp strings being interpreted in the execution timezone, which is reasonable. Dropping the timezone of inputs that are already zoned appears to be an unintended side effectas it makes a normalisation function silently discard a caller's explicit timezone.
### To Reproduce
Run in `datafusion-cli` with default settings (no `SET datafusion.execution.time_zone`):
```sql
-- input epoch = 2024-01-01T00:00:00Z, annotated +05:00 => local wall clock 05:00
SELECT
arrow_typeof(to_timestamp_millis(arrow_cast(1704067200000, 'Timestamp(Millisecond, Some("+05:00"))'))) AS result_type,
date_part('hour', arrow_cast(1704067200000, 'Timestamp(Millisecond, Some("+05:00"))')) AS hour_direct,
date_part('hour', to_timestamp_millis(arrow_cast(1704067200000, 'Timestamp(Millisecond, Some("+05:00"))'))) AS hour_via_to_timestamp;
```
**Actual**
| result_type | hour_direct | hour_via_to_timestamp |
|---|---|---|
| `Timestamp(Millisecond, None)` | 5 | 0 |
**Befor the change**
| result_type | hour_direct | hour_via_to_timestamp |
|---|---|---|
| `Timestamp(Millisecond, Some("+05:00"))` | 5 | 5 |
### Expected behavior
`to_timestamp_millis` (and the rest of the family) should not silently drop the timezone of an already-zoned input. Passing a `Timestamp(_, Some("+05:00"))` through the function should either:
1. preserve the input zone, or
2. if honoring `datafusion.execution.time_zone`, convert the timezone but never relabel to a naive type, which changes the wall-clock value.
In particular, when `datafusion.execution.time_zone` is unset (`None`), a zoned input should keep its zone rather than becoming naive. As it stands, `date_part('hour', to_timestamp_millis())` returns the UTC hour instead of the local hour, which corrupts any hour-of-day / day-of-month bucketing over zoned columns.
### Additional context
Relevant commit: `ada0923a3927d16dc5d810637cfbea4146c54f54` (PR #19078 that closed #17998)
Contributor guide
Research direction
Start by tracing the to_timestamp, to_timestamp_seconds, to_timestamp_millis, to_timestamp_micros, and to_timestamp_nanos implementations, especially the generic Timestamp match arm changed by commit ada0923a3927d16dc5d810637cfbea4146c54f54. Reproduce the issue in datafusion-cli with the provided zoned timestamp query, then verify that the result preserves the input timezone and that date_part returns the local hour.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 65/100