Resolve DST-ambiguous and nonexistent local times when casting timestamps to a named timezone - #11038
Resolve DST-ambiguous and nonexistent local times when casting timestamps to a named timezone#11038adriangb wants to merge 2 commits into
Conversation
…amps to a named timezone
Casting `Timestamp(_, None)` to `Timestamp(_, Some(tz))` reads each value as a
wall clock time in `tz`. In an IANA timezone that reading does not always
identify a unique instant:
* When the clocks go back ("fall back") the hour repeats and the reading is
ambiguous.
* When the clocks go forward ("spring forward") an hour is skipped and the
reading never occurs.
`adjust_timestamp_to_timezone` used `LocalResult::single()`, which is `None` for
both cases, so values on either kind of DST boundary failed with "Cannot cast
timezone to different timezone" under `CastOptions { safe: false }` and silently
became NULL under `safe: true`.
Resolve them instead, matching PostgreSQL 17 and DuckDB 1.5:
* Ambiguous readings take the *later* instant, i.e. the offset in effect after
the transition. `2024-11-03T01:30:00` in `America/New_York` becomes
`01:30-05:00` (EST).
* Nonexistent readings are shifted forward by the length of the gap, which is
the same as reading them with the offset in effect *before* the transition.
`2024-03-10T02:30:00` in `America/New_York` becomes `03:30-04:00` (EDT).
The pre-transition offset is recovered by probing the same lookup 24 hours
earlier and taking the earliest result; the timezone database contains no two
transitions within 24 hours of each other, so the probe always lands on the
other side of the transition. `offset_from_utc_datetime` is not usable as a
shortcut here: it returns the post-transition offset, which is wrong for
southern hemisphere gaps such as `Australia/Sydney` `2024-10-06T02:30:00`.
Fixed-offset timezones have no transitions and are unaffected. Unit conversion,
`safe` handling and the error message are unchanged.
Tests live in `arrow-cast`, which now enables `arrow-array/chrono-tz` as a
dev-dependency so its own tests can use IANA timezone names. The one existing
assertion on the "invalid timezone" parser message is relaxed to its common
prefix, since the tail of that message depends on whether `chrono-tz` is on.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ad46575 to
6458760
Compare
`to_local_time(t) AT TIME ZONE 'zone'` currently raises "Cannot cast timezone to different timezone" across an ambiguous hour, so the round trip cannot silently return a different instant. apache/arrow-rs#11038 removes that error, and once DataFusion picks it up the round trip moves by an hour with no error and no warning. PostgreSQL behaves the same way, so this is the intended trade rather than a regression -- but today the error is what stops users writing that round trip, so the docs need to say so before the behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Self-review QA pass — this is our own PR, so treat this as our own adversarial check, not an independent review. No blocking defect. I re-measured every claim. Four findings, in priority order. 1. The stated safety argument for the 24-hour probe is not the load-bearing oneThe PR justifies the probe with a transition-spacing figure: no two transitions in 597 zones are closer than 167 hours. That figure is correct. I reproduce it exactly ( Two separate invariants are at work here:
Seven zones have a local gap of exactly 24.00 hours. These are the dateline changes:
The next largest gap is 10.00 h ( For a reading at the last second of such a gap the probe lands one second before the gap starts. The real margin is one second, not 167 hours. The code is correct today. It stops being correct the day tzdb gains a gap above 24 hours. I tested all four surviving dateline zones at the first instant of the gap, the middle and the last second. All 13 readings match PostgreSQL 17.11 exactly: Please state the gap-length invariant next to the spacing invariant, both in the PR body and in the doc comment on 2. The give-up branch is unreachable, so its callers are untested
So the branch is dead code with real tzdata. I read the callers instead of testing them, and both are correct:
The I confirmed the two options stay in step on real DST input: 3. The fixed-offset claim is over-broadThe PR says PostgreSQL reads a fixed-offset string such as
So PostgreSQL agrees with arrow-rs when the offset sits inside the literal, and disagrees when the offset acts as a zone name. DuckDB 1.5.2 rejects the zone-name spelling outright ( 4. The dev-dependency removes the no-
|
| Wall clock reading | arrow | PostgreSQL 17.11 |
|---|---|---|
2024-01-15 12:00:00 |
1705338000 | 1705338000 |
2024-03-10 01:30:00 |
1710052200 | 1710052200 |
2024-03-10 02:30:00 (gap) |
1710055800 | 1710055800 |
2024-03-10 03:30:00 |
1710055800 | 1710055800 |
2024-06-15 12:00:00 |
1718467200 | 1718467200 |
2024-11-03 00:30:00 |
1730608200 | 1730608200 |
2024-11-03 01:30:00 (ambiguous) |
1730615400 | 1730615400 |
2024-11-03 02:30:00 |
1730619000 | 1730619000 |
2024-12-15 12:00:00 |
1734282000 | 1734282000 |
Three distinct offsets appear in one array. A single offset per array cannot produce this.
A broad independent sweep agrees. I generated every DST transition of every zone from 2020 to 2025, then sampled the start and the midpoint of each gap and each repeated interval. That gives 5,012 readings across 226 zones: 2,506 in a gap, 2,506 ambiguous. Results:
- 0 unresolved readings under
safe: true. - 3,908 of them use a zone PostgreSQL 17.11 accepts. 16 disagree.
- All 16 are
Europe/Chisinauand its aliasEurope/Tiraspol.
That is the chrono-tz tzdata skew the PR already discloses. I confirmed it predates this PR. On the base commit, with no change applied:
main Europe/Chisinau 2024-03-31 03:30 -> 1711845000
PostgreSQL 17.11 -> 1711848600
DuckDB 1.5.2 -> 1711848600
PostgreSQL and DuckDB agree with each other, so chrono-tz is the outlier. It puts the Moldova spring transition at 02:00 local; the other two put it at 03:00 local. Non-boundary readings agree everywhere. Please add Europe/Tiraspol to the disclosure — it carries the same skew and the PR names only Europe/Chisinau.
The known divergence stays untouched and disclosed. git diff against the base commit shows this branch changes nothing under arrow-array/. arrow-array/src/types.rs:349 still reads Ambiguous(dt1, _) and None => None. Measured side by side in America/New_York:
2024-11-03 01:30:00 |
Result |
|---|---|
from_naive_datetime |
1730611800 = 05:30Z, the earlier instant |
| this PR's cast | 1730615400 = 06:30Z, the later instant |
Opposite, as the PR states. The disclosure in the body is accurate and prominent.
Test coverage. The six new tests cover the southern-hemisphere gap (Australia/Sydney), both safe modes, the nanosecond path, a unit change and a fixed offset. Australia/Lord_Howe, Pacific/Chatham, America/Sao_Paulo and America/Havana are not in this PR's tests, but #11054 adds them. I verified all four against PostgreSQL by hand and they pass on this branch too.
Suite. cargo test -p arrow-cast passes on this branch.
The comment gave only one reason the probe is sound: that no two transitions are closer than 24 hours. That is the wrong invariant for the question it was answering. It explains why the offset the probe finds is the *correct* one, but not why the probe lands somewhere *resolvable*. What makes the probe resolvable is a different property: no local gap is longer than 24 hours. Seven zones have a gap of exactly 24 hours -- the dateline changes, such as Pacific/Apia in 2011 and Pacific/Kiritimati in 1994. At the last second of one of those, the probe lands one second before the gap starts. So the real margin is one second, not the 167 hours the old comment implied. Verified against tzdata directly: over 598 zones and 20470 forward transitions between 1900 and 2040, zero gaps exceed 24 hours, seven are exactly 24 hours, and the next largest is 10 hours. Comment only, no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Which issue does this PR close?
Timestamp(_, None)to a named timezone fails on DST-ambiguous and nonexistent local times #11037.Rationale for this change
What a user hits today
Cast a naive timestamp to a named timezone. Pick a wall clock reading that the zone skips or repeats. The cast fails.
Today:
After this PR:
The
safe: trueresult is the dangerous one. A whole column of DST-boundary readings turns to NULL, and nothing reports it.How this reaches a SQL user
A DataFusion user reaches the same kernel through ordinary SQL. This is how the bug was found:
The engine plans that as
Timestamp(_, None)toTimestamp(_, Some("America/New_York")), so the error text above comes straight from this kernel. See apache/datafusion#25084. An upcoming type-coercion change inserts the same cast automatically fortimestamptz - timestamp, so the failure gets more common, not less.Why the kernel fails
adjust_timestamp_to_timezonereads each value as a wall clock time in the target zone. In an IANA timezone that reading is not always one instant:The kernel resolved the offset with
offset_from_local_datetime(..).single(). That isNonefor both cases, so both cases fail.What changes are included in this PR?
A new private helper,
resolve_local_offset, decides the two cases instead of rejecting them. It follows PostgreSQL and DuckDB:2024-11-03T01:30:00inAmerica/New_Yorkbecomes01:30-05:00(EST).2024-03-10T02:30:00inAmerica/New_Yorkbecomes03:30-04:00(EDT).adjust_timestamp_to_timezonecalls the helper in place of.single(). That is the whole behavioural change.The forward shift needs the offset in effect before the transition. The code recovers it with one probe: it asks for the offset 24 hours earlier and takes the earliest result.
offset_from_utc_datetimeis not a valid shortcut, because it returns the post-transition offset. That is wrong for a southern hemisphere gap such asAustralia/Sydney2024-10-06T02:30:00.Two properties of the timezone database make the probe safe:
America/Boa_Vistain 2000.Pacific/Apiain 2011. The next largest gap is 10 hours.If the probe still fails, the helper returns
Noneand the caller keeps the old error or NULL. With current tzdata that branch is unreachable.Nothing else changes. Unit conversion,
safehandling and the error message stay as they were. A fixed-offset timezone has no transitions, so it is unaffected.Are these changes tested?
Yes. Six new tests sit next to the existing
test_cast_timestamp_with_timezone_*tests inarrow-cast/src/cast/mod.rs:test_cast_timestamp_to_named_timezone_dst—America/New_York, with an unambiguous, an ambiguous and a nonexistent reading plus a null, undersafe: false.test_cast_timestamp_to_named_timezone_dst_safe— the same input undersafe: true, which no longer produces nulls.test_cast_timestamp_to_named_timezone_dst_southern_hemisphere—Australia/Sydney, where the transitions run the other way round.test_cast_timestamp_to_named_timezone_dst_nanosecondand..._dst_changing_unit— the resolution composes with the unit paths.test_cast_timestamp_to_fixed_offset_timezone_unaffected—+08:00on the same readings is unchanged.Expected values come from
chrono, built from the wall clock reading and the expected offset. They are not hardcoded. Revert the one-line kernel change and the five DST tests fail, while the fixed-offset test still passes.Validation
The change was checked against 97,162 cases. These cover every transition of every IANA zone in 1890–1995 and 2023–2025: 15,871 in a gap and 14,370 ambiguous. There were zero disagreements with an independent implementation on the same tzdata.
Offsets resolve per row. One array that straddles both 2024
America/New_Yorktransitions yields three distinct offsets, and every element matches PostgreSQL 17.11.One unrelated discrepancy came up. chrono-tz 0.10.4 bundles tzdata 2025b, which puts the
Europe/ChisinauandEurope/Tiraspoltransitions an hour away from PostgreSQL, ICU and CPython. That reproduces onmainwithout this PR. It needs a chrono-tz bump, not a change here.A note on the test manifest
arrow-cast had no way to name an IANA zone in its own tests.
Tzparses IANA names only whenarrow-arrayis built withchrono-tz, and arrow-cast never enabled it.This PR enables it on the dev-dependency only:
arrow-array = { workspace = true, features = ["chrono-tz"] }under[dev-dependencies]. The new tests then run in every arrow-cast CI job. No public feature appears, and nothing changes for downstream crates.One test needed an update.
test_cast_string_to_timestamp_invalid_tzasserted the exact error tailonly offset based timezones supported without chrono-tz feature, which is nowfailed to parse timezone. It asserts the stable prefixParser error: Invalid timezone \"ZZTOP\":instead.Local runs that pass:
cargo test -p arrow-cast(default and--all-features, debug and release),cargo test -p arrow --features chrono-tz,prettyprint --test array_cast --test timezone, clippy with-D warningson the CI feature combinations, andcargo doc --all-featureswith-D warnings.Field research: PostgreSQL 17 and DuckDB
The resolution policy is the one arbitrary decision in this change, so it needs more than one reference. Both engines were measured directly: PostgreSQL 17.11 in
postgres:17, and the DuckDB 1.5.2 CLI with the ICU extension. All times below are UTC.America/New_York2024-11-03 01:30— ambiguous2024-11-03 06:30:002024-11-03 06:30:002024-11-03 06:30:00America/Havana2024-11-03 00:00— ambiguous local midnight2024-11-03 05:00:002024-11-03 05:00:002024-11-03 05:00:00America/New_York2024-03-10 02:30— gap2024-03-10 07:30:002024-03-10 07:30:002024-03-10 07:30:00America/Sao_Paulo2018-11-04 00:00— local midnight does not exist2018-11-04 03:00:002018-11-04 03:00:002018-11-04 03:00:00Australia/Sydney2024-10-06 02:30— southern hemisphere gap2024-10-05 16:30:002024-10-05 16:30:002024-10-05 16:30:00Australia/Lord_Howe2024-10-06 02:15— 30 minute DST step2024-10-05 15:45:002024-10-05 15:45:002024-10-05 15:45:00Pacific/Chatham2024-09-29 03:00— +12:45 / +13:452024-09-28 14:15:002024-09-28 14:15:002024-09-28 14:15:00The two ambiguous rows carry the argument. A gap has only one sensible answer, so it discriminates nothing. An ambiguous reading has two real answers, and the choice between them is the policy.
2024-11-03 01:30in New York is either05:30Z(EDT) or06:30Z(EST). Both engines give06:30Z. Havana's ambiguous midnight is either04:00Z(CDT) or05:00Z(CST). Both engines give05:00Z. In both cases the answer is the later instant, which is what this PR does.The case neither engine can arbitrate
A fixed-offset string such as
'+05:30'has no agreed reading, so it is recorded here rather than hidden. Measured:SET TimeZone='+05:30'then'2024-01-01 12:00:00'::timestamptz17:30Z'2024-01-01 12:00:00'::timestamp AT TIME ZONE '+05:30'17:30Ztimestamptz '2024-01-01 12:00:00 +05:30'06:30ZPostgreSQL agrees with arrow-rs when the offset sits inside the literal. It disagrees when the offset acts as a zone name. DuckDB 1.5.2 rejects the zone-name spelling outright:
Not implemented Error: Unknown TimeZone '+05:30'.So neither engine settles it. This PR does not change that behaviour. It is tracked downstream in apache/datafusion#25170.
Are there any user-facing changes?
Yes. A cast that previously raised an error, or returned NULL under
safe: true, now returns the instants described above. There is no API change.This PR does not close DataFusion #25084 on its own
There are two independent paths to a DST-boundary failure. This PR fixes one of them:
To a user those are the same query, and the last spelling is the most natural one. Merge this alone and an explicit
::timestampin the middle of an expression makes a query start to work. That is harder to explain than the current uniform failure. #11039 must land with this PR or close behind it, and must use the same policy so the two paths cannot drift.This PR is sufficient for apache/datafusion#10308, which is entirely on the cast path.
Follow-up:
arrow-arrayresolves ambiguity the opposite wayAfter this PR, arrow resolves an ambiguous local time two different ways in two crates:
arrow-cast::resolve_local_offset(this PR)arrow-array::types::from_naive_datetime(types.rs:347)Ambiguous(dt1, _))NoneThat function is deliberately untouched here.
This matters downstream. DataFusion calls the
arrow-arrayform directly with aSome(tz)indatafusion/functions/src/datetime/date_part.rs(date_to_scalar, four call sites). It builds local-midnight bounds for thedate_part(YEAR, col) = <year>rewrite. Where that local midnight is ambiguous, the earlier instant becomes an upper bound, so valid rows can drop out. The behaviour predates this PR, but this PR widens the gap between the two policies.The two want reconciling, or the difference wants documenting on both functions. Changing
from_naive_datetimeaffects every caller ofarrow-array, so it deserves its own review rather than a ride along with a cast fix.Follow-up: a loud error becomes a quiet non-identity
to_local_time(ts) AT TIME ZONE tzis a documented DataFusion idiom. Across an ambiguous hour it is no longer the identity, and it no longer says so. Before this PR the second leg raisedCannot cast timezone to different timezone:PostgreSQL behaves the same way.
t = ((t AT TIME ZONE 'Europe/Brussels') AT TIME ZONE 'Europe/Brussels')isffor those same two instants. So this is the intended trade, not an argument against the change. But it is a real behaviour change, and downstream docs want to say so.🤖 Generated with Claude Code