apache / apache/datafusion-comet

timestamp_trunc panics on DST-transition timestamps in a DST timezone

Open
#5,633 0 comments 0 reactions 1 assignee Claimed by @coderfender View on GitHub
requires-triage
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.