Skip to content

fix: truncate timestamps around DST transitions the way Spark does - #6354

Open
andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:fix-timestamp-trunc-dst
Open

andygrove wants to merge 1 commit into
apache:mainfrom
andygrove:fix-timestamp-trunc-dst

Conversation

@andygrove

@andygrove andygrove commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #5633.

Rationale for this change

The timezone-aware path of the native date_trunc built the truncated time with chrono's with_hour, with_minute, with_day0 and with_month0 on a DateTime<Tz>. Those return None when the local result is ambiguous (fall-back) or falls in a gap (spring-forward). as_micros_from_unix_epoch_utc then unwrapped the None, so truncating a timestamp near a DST transition panicked. trunc_timestamp_dst_ambiguous.sql had every query marked ignore because of this.

Even where it didn't panic, the old path didn't follow Spark for ambiguous results. Spark's DateTimeUtils.truncTimestamp treats the levels differently:

Level Spark Ambiguous local result Local result in a gap
MICROSECOND, MILLISECOND, SECOND truncates the instant (offsets are whole seconds) n/a n/a
MINUTE, HOUR, DAY ZonedDateTime.truncatedTo keeps the input's offset if it is still valid moved forward by the gap's length
WEEK, MONTH, QUARTER, YEAR truncate the local date, then LocalDate.atStartOfDay earlier offset first instant after the gap

So on the 2024-11-03 fall-back in America/Los_Angeles, the second 01:30 (PST) truncates to 01:00 PST, not to the first 01:00 (PDT).

What changes are included in this PR?

  • The timezone-aware truncation now works on the local time. It truncates the naive local datetime with the same helpers the TIMESTAMP_NTZ path uses, then resolves it back to an instant with the rule for its level from the table above. A gap takes the offset from before the gap through resolve_local_datetime, the helper the date-to-timestamp cast already uses. That gives the same instant as Java's rule.
  • MICROSECOND, MILLISECOND and SECOND truncate the instant directly, as Spark does, instead of going through the calendar.
  • as_micros_from_unix_epoch_utc and the DateTime<Tz>-based helpers are removed.
  • A new Rust unit test checks 49 truncations against values computed with java.time, which is what Spark's truncTimestamp calls. It covers both 01:30s of the 2024-11-03 Los Angeles fall-back, the 2024-03-10 spring-forward, the 1970-10-25 repro from the issue, São Paulo's skipped midnight on 2018-11-04, and both 23:30s of its 2019-02-16 fall-back.
  • trunc_timestamp_dst_ambiguous.sql runs its six queries again. It adds the second occurrence of the ambiguous hour and a MINUTE query.
  • A new trunc_timestamp_dst_midnight.sql covers São Paulo's midnight transitions.

This conflicts with #5956, which also reworks this kernel.

The serde still marks truncation in non-UTC sessions Incompatible, so by default it still runs through the codegen dispatcher. This PR fixes the native path that allowIncompatible opts into. The Incompatible reason still cites #2649, which #4761 closed. Two known divergences remain: chrono-tz's DST rules end in 2099, which the datetime compatibility guide documents, and tzdata can drift from the JVM's (#6331). So I left the support level alone.

@coderfender, I picked this up as part of the timezone EPIC (#6335). Happy to hand it back if you already have work in progress.

How are these changes tested?

  • Old kernel: the new unit test panics at temporal.rs:166. Both trunc_timestamp_dst_ambiguous.sql and trunc_timestamp_dst_midnight.sql fail with the native panic.
  • With this change:
    • Both SQL files match Spark.
    • All expressions/datetime/ SQL file tests and CometTemporalExpressionSuite pass on Spark 4.1.
    • The new and existing kernel unit tests pass.
    • cargo clippy --all-targets -- -D warnings is clean for datafusion-comet-spark-expr.

The timezone-aware date_trunc path built the truncated time with chrono's with_hour/with_minute/with_day0 on DateTime<Tz>, which return None when the local result is ambiguous or falls in a gap, and the kernel unwrapped that None and panicked. Truncate the local time instead and resolve it the way Spark's DateTimeUtils.truncTimestamp does: MINUTE, HOUR and DAY keep the input's offset when the result is ambiguous, as ZonedDateTime.truncatedTo does, the date levels use the earlier offset, as LocalDate.atStartOfDay does, and SECOND and finer truncate the instant.

Closes apache#5633.
@andygrove andygrove added backport-1.0 Candidate for backporting to 1.0 release branch backport-1.1 Candidate for backporting to 1.1 release branch labels Sep 28, 2026
@github-actions github-actions Bot added bug Something isn't working area:expressions Expression evaluation labels Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:expressions Expression evaluation backport-1.0 Candidate for backporting to 1.0 release branch backport-1.1 Candidate for backporting to 1.1 release branch bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

timestamp_trunc panics on DST-transition timestamps in a DST timezone

1 participant