apache / apache/datafusion-comet
timestamp_trunc panics on DST-transition timestamps in a DST timezone
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
### Describe the bug
`TimestampTrunc` / `timestamp_trunc` panics (`called \`Option::unwrap()\` on a \`None\` value`) when truncating a timestamp that lands on (or near) a daylight-saving-time transition, in a session timezone that observes DST (e.g. `America/Los_Angeles`).
The panic is in the truncation kernel:
`native/spark-expr/src/kernels/temporal.rs:166`
```rust
fn as_micros_from_unix_epoch_utc(dt: Option>) -> i64 {
let dt = dt.unwrap(); // <-- panics
...
}
```
Root cause: the `trunc_date_to_*` helpers build the truncated time with chrono's `with_hour(0)` / `with_minute(0)` / `with_day0(0)` / `with_month0(0)`. On a `DateTime`, those methods return `None` when the resulting **local** time is ambiguous (fall-back) or nonexistent (spring-forward). The kernel then `unwrap()`s that `None` instead of resolving it the way `as_micros_from_unix_epoch_utc` already does for `LocalResult::None`.
Spark does not crash on these inputs.
### To Reproduce
With `spark.sql.session.timeZone = America/Los_Angeles` and Comet enabled:
```sql
-- 1970-10-25 was a US DST fall-back date; the ambiguous local hour trips the panic
SELECT date_trunc('HOUR', TIMESTAMP '1970-10-25 01:30:00');
SELECT date_trunc('YEAR', TIMESTAMP '1970-10-25 09:39:00');
```
Rust-level repro (what surfaced it): a `Timestamp(Microsecond, Some("America/Los_Angeles"))` array with representative sub-day timestamps, truncated to `YEAR`/`QUARTER`/`MONTH`/`DAY`/`HOUR`, panics. A scan over 8192 representative values:
```
America/Los_Angeles YEAR/QUARTER/MONTH/DAY/HOUR -> panics
America/Phoenix, Asia/Kolkata, UTC (no DST) -> clean
```
`WEEK` does not panic (its chain subtracts a `Duration` rather than re-resolving through the tz).
### Expected behavior
Match Spark: resolve ambiguous/gap local times (pick a deterministic offset, as `as_micros_from_unix_epoch_utc` already does for `LocalResult::None`) instead of `unwrap()`ing `None`. No panic.
### Additional context
Surfaced while making the datetime/timezone Criterion benches more representative (PR #5620, part of #5396). That PR sidesteps the panic by benching `timestamp_trunc` with a fixed-offset no-DST timezone (`Asia/Kolkata`); this issue tracks the underlying kernel fix.
Contributor guide
Assessment
This issue has not been assessed yet.