adriangb opened a new pull request, #11038:
URL: https://github.com/apache/arrow-rs/pull/11038
# Which issue does this PR close?
- Closes #11037.
# Rationale for this change
Casting `Timestamp(_, None)` to a **named** timezone reads each value as a
wall-clock time in that zone. Around a daylight saving transition two readings
are not 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. `adjust_timestamp_to_timezone` resolves the offset with
`offset_from_local_datetime(..).single()`, which is `None` in both cases, so
the cast fails with `Cannot cast timezone to different timezone` under `safe:
false` and silently produces `NULL` under `safe: true`:
```
safe=false: Err(CastError("Cannot cast timezone to different timezone"))
safe=true: ["2024-11-01T00:00:00-04:00", "", ""]
```
PostgreSQL 17.11 and DuckDB 1.5.2 both resolve these deterministically and
agree with each other exactly:
```sql
SET TimeZone = 'America/New_York';
SELECT '2024-11-03T01:30:00'::timestamp::timestamptz AS ambiguous,
'2024-03-10T02:30:00'::timestamp::timestamptz AS nonexistent;
-- ambiguous | nonexistent
-- ------------------------+------------------------
-- 2024-11-03 01:30:00-05 | 2024-03-10 03:30:00-04
```
This surfaced through DataFusion
(https://github.com/apache/datafusion/issues/25084), where an upcoming
type-coercion change will insert this cast automatically for `timestamptz -
timestamp`.
# What changes are included in this PR?
`adjust_timestamp_to_timezone` now resolves the two cases the way PostgreSQL
and DuckDB do:
- **ambiguous** local times resolve to the **later** instant, i.e. the
post-transition (standard) offset: `2024-11-03T01:30:00` in `America/New_York`
becomes `2024-11-03T01:30:00-05:00`;
- **nonexistent** local times shift **forward** by the size of the gap,
which is the same as interpreting the reading with the pre-transition offset:
`2024-03-10T02:30:00` in `America/New_York` becomes
`2024-03-10T03:30:00-04:00`. The pre-transition offset is recovered by probing
the offset in effect 24 hours earlier (tzdb has no two transitions within a day
of each other); if even that cannot be resolved the existing error/NULL
behaviour is kept.
Nothing else changes: unit conversion, `safe` handling and the error message
are as before, and fixed-offset timezones are unaffected.
I did not add a policy to `CastOptions` (Arrow C++'s `AssumeTimezoneOptions`
offers raise/earliest/latest for each case): `CastOptions` is a plain public
struct, so a new field is a breaking change and would rule out a patch release.
If someone needs `earliest` or `raise`, that can be added in a major release on
top of this default.
`string_to_datetime` in `parse.rs` has the same `.single()` pattern for
strings carrying a named zone; that is a generic-`TimeZone` code path and is
left for a follow-up.
# Are these changes tested?
Yes. Six new tests in `arrow-cast/src/cast/mod.rs`, next to the existing
`test_cast_timestamp_with_timezone_*` tests:
- `test_cast_timestamp_to_named_timezone_dst`: `America/New_York`, an
unambiguous, an ambiguous and a nonexistent reading plus a null, `safe: false`;
asserts the output type and the resolved instants.
- `test_cast_timestamp_to_named_timezone_dst_safe`: the same input with
`safe: true` 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_nanosecond` and
`..._dst_changing_unit` (`Second` naive → `Millisecond` with timezone): the
resolution composes with the unit paths.
- `test_cast_timestamp_to_fixed_offset_timezone_unaffected`: `+08:00` on the
same wall-clock values is unchanged.
Expected values are built with `chrono` from the wall-clock reading and the
expected offset rather than hardcoded. With the one-line kernel change
reverted, the five DST tests fail and the fixed-offset test still passes.
arrow-cast had no way to exercise a named timezone in its tests: `Tz` only
parses IANA names when `arrow-array` is built with `chrono-tz`, which
arrow-cast never enabled. This PR enables it on the **dev-dependency only**
(`arrow-array = { workspace = true, features = ["chrono-tz"] }` under
`[dev-dependencies]`), so the new tests run in every arrow-cast CI job without
adding a public feature or changing what downstream crates build. One
consequence: `test_cast_string_to_timestamp_invalid_tz` asserted the exact
error tail `only offset based timezones supported without chrono-tz feature`,
which is now `failed to parse timezone`; it now asserts the stable prefix
`Parser error: Invalid timezone \"ZZTOP\":` instead.
Locally: `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 warnings` on the CI feature
combinations, and `cargo doc --all-features` with `-D warnings` all pass.
# Are there any user-facing changes?
Yes: casts that previously errored (or returned `NULL` with `safe: true`)
for DST-boundary wall-clock values now return the instants described above. No
API changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
