fix: resolve DST-ambiguous and nonexistent local times when casting naive timestamps to a named timezone - #25115
Closed
adriangb wants to merge 1 commit into
Closed
fix: resolve DST-ambiguous and nonexistent local times when casting naive timestamps to a named timezone#25115adriangb wants to merge 1 commit into
adriangb wants to merge 1 commit into
Conversation
…aive timestamps to a named timezone
Casting a `Timestamp(_, None)` to a `Timestamp(_, Some(tz))` means reading a
wall clock time in `tz`. Around a daylight saving transition that reading is
not always a single instant: the hour repeated by a "fall back" transition is
ambiguous, and the hour skipped by a "spring forward" transition does not
exist. arrow's cast kernel resolves the offset with
`offset_from_local_datetime(..).single()`, which is `None` in both cases, so
these casts fail with `Cannot cast timezone to different timezone` (or produce
NULL under `TRY_CAST`).
PostgreSQL and DuckDB resolve both deterministically, and DataFusion now does
the same, in a new `datafusion_common::timezone_cast` module wired into the two
DataFusion cast entry points (`ColumnarValue::cast_to` and
`ScalarValue::cast_to_with_options`) for exactly that one pair of types:
* ambiguous local times resolve to the later instant, i.e. the
post-transition (standard) offset, so `2024-11-03T01:30:00` in
`America/New_York` is `2024-11-03T01:30:00-05:00`;
* nonexistent local times shift forward by the size of the gap, so
`2024-03-10T02:30:00` in `America/New_York` is `2024-03-10T03:30:00-04:00`.
Everything else, including the unit conversion, is still delegated to arrow.
Closes apache#25084
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Closing this: the right place for the fix is arrow-rs, where |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Timestamp(_, None)to a named timezone errors on DST boundaries #25084.Rationale for this change
Casting a timezone-naive timestamp to a named timezone fails whenever the wall clock value falls on a daylight saving boundary:
TRY_CASTturns the same values intoNULL. Unambiguous times and fixed-offset timezones are unaffected. It reproduces on real columns as well as literals.arrow-cast resolves the offset with
offset_from_local_datetime(..).single(), which isNonefor bothLocalResult::AmbiguousandLocalResult::None. PostgreSQL 17.11 and DuckDB 1.5.2 both resolve these deterministically, and they agree with each other:America/New_York)2024-11-03T01:30:00(ambiguous)2024-11-03 01:30:00-052024-11-03T01:30:00-05:002024-03-10T02:30:00(nonexistent)2024-03-10 03:30:00-042024-03-10T03:30:00-04:00Reference behaviour (same SQL on PostgreSQL 17.11 and DuckDB 1.5.2)
PostgreSQL 17.11:
DuckDB 1.5.2:
DataFusion with this PR (
SET datafusion.execution.time_zone = 'America/New_York'):This matters beyond explicit casts: the fix for #13212 (see #25094) makes type coercion insert exactly this cast for
timestamptz - timestamp, so without this change any such query under a named session timezone starts erroring on DST-boundary rows.What changes are included in this PR?
datafusion_common::timezone_castimplementing the PostgreSQL/DuckDB convention:ColumnarValue::cast_to(arrays, i.e.CastExpr) andScalarValue::cast_to_with_options(scalars, i.e. constant folding), route the one type pairTimestamp(_, None) -> Timestamp(_, Some(tz))through it. Everything else, including the unit conversion andCastOptions::safehandling, is still delegated to arrow's kernel, and the error message for a value that still cannot be resolved is unchanged.I chose to do this in DataFusion rather than arrow-rs because which instant an ambiguous wall clock time maps to is a SQL-engine semantic choice (matching PostgreSQL/DuckDB) rather than something arrow's kernel should decide by default. If arrow-rs later grows a
CastOptionsknob for this, the module can shrink to a call into it.What is the testing strategy for this PR?
datafusion/common/src/timezone_cast.rs: unambiguous, ambiguous and gap values forAmerica/New_YorkandAustralia/Sydney(southern hemisphere, opposite transition order), a fixed offset, null preservation, a unit-changing cast, and equality witharrow::compute::castfor values where arrow already succeeds.datafusion/sqllogictest/test_files/datetime/cast_timestamp_dst.slt: the reproductions from the issue as literals (constant-folded scalar path) and as table columns (array path), all four time units,TRY_CAST, NULL inputs,to_unixtimechecks of the resolved instants,Australia/Sydney, and a fixed-offset+08:00control showing unchanged behaviour. Expected epoch seconds were computed independently rather than copied from the runner.statement errorforTIMESTAMPTZ '2023-03-12 02:00:00 America/Los_Angeles'is untouched: that is the string parser, not the cast.Are there any user-facing changes?
Yes, in the sense that casts which used to error (or return
NULLunderTRY_CAST) now return a value, following the PostgreSQL/DuckDB convention described above. No public API changes.🤖 Generated with Claude Code