apache / apache/arrow-rs

Casting `Timestamp(_, None)` to a named timezone fails on DST-ambiguous and nonexistent local times

Open
#11,037 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 18h
Merged PRs (30d)
169

Description

**Describe the bug**

Casting a `Timestamp(_, None)` array to a `Timestamp(_, Some(tz))` with a **named** timezone fails for every value whose wall-clock reading falls on a daylight saving transition:

- the hour repeated by a "fall back" transition (ambiguous), and
- the hour skipped by a "spring forward" transition (nonexistent).

With `CastOptions { safe: false }` the whole cast fails with `Cast error: Cannot cast timezone to different timezone`; with `safe: true` the affected values silently become `NULL`. Unambiguous readings and fixed-offset timezones (`+08:00`) are fine.

The cause is `adjust_timestamp_to_timezone` in `arrow-cast/src/cast/mod.rs`:

```rust
let adjust = |o| {
let local = as_datetime::(o)?;
let offset = to_tz.offset_from_local_datetime(&local).single()?;
T::from_naive_datetime(local - offset.fix(), None)
};
```

`LocalResult::single()` is `None` for both `LocalResult::Ambiguous` and `LocalResult::None`, so both cases collapse into the failure path.

**To Reproduce**

arrow-cast 59.3.0, arrow-array with the `chrono-tz` feature:

```rust
use std::sync::Arc;
use arrow_array::{Array, ArrayRef, TimestampSecondArray};
use arrow_cast::{cast_with_options, CastOptions};
use arrow_schema::{DataType, TimeUnit};

fn main() {
// Naive wall-clock readings, to be interpreted in America/New_York:
// 2024-11-01T00:00:00 unambiguous
// 2024-11-03T01:30:00 ambiguous (fall back: 01:30 happens twice)
// 2024-03-10T02:30:00 nonexistent (spring forward: 02:30 never happens)
let naive: ArrayRef = Arc::new(TimestampSecondArray::from(vec![
1_730_419_200, 1_730_597_400, 1_710_037_800,
]));
let to = DataType::Timestamp(TimeUnit::Second, Some("America/New_York".into()));

let strict = CastOptions { safe: false, ..Default::default() };
println!("safe=false: {:?}", cast_with_options(&naive, &to, &strict).map(|a| a.len()));

let lenient = CastOptions { safe: true, ..Default::default() };
let out = cast_with_options(&naive, &to, &lenient).unwrap();
let shown: Vec = (0..out.len())
.map(|i| arrow_cast::display::array_value_to_string(&out, i).unwrap())
.collect();
println!("safe=true: {shown:?}");
}
```

```
safe=false: Err(CastError("Cannot cast timezone to different timezone"))
safe=true: ["2024-11-01T00:00:00-04:00", "", ""]
```

**Expected behavior**

Both values resolve to an instant instead of failing. PostgreSQL 17.11 and DuckDB 1.5.2 agree exactly on how:

```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
```

- **Ambiguous**: pick the **later** instant, i.e. the post-transition (standard) offset. In chrono terms `LocalResult::Ambiguous(_, later)` → `later`.
- **Nonexistent**: shift **forward** by the size of the gap, which is the same as interpreting the reading with the pre-transition offset.

So `[1_730_419_200, 1_730_597_400, 1_710_037_800]` should become the instants `[1_730_433_600, 1_730_615_400, 1_710_055_800]`, displayed as `2024-11-01T00:00:00-04:00`, `2024-11-03T01:30:00-05:00`, `2024-03-10T03:30:00-04:00`.

**Reference implementations (source)**

The rule above was checked against the reference engines' source, not only their output:

- **PostgreSQL** `DetermineTimeZoneOffsetInternal`, [`src/backend/utils/adt/datetime.c` L1703-L1721 (REL_17_STABLE)](https://github.com/postgres/postgres/blob/bee33974ee726785a674fafd1c58f8d0cd9c6667/src/backend/utils/adt/datetime.c#L1703-L1721): "It's an invalid or ambiguous time due to timezone transition. In a spring-forward transition, prefer the 'before' interpretation; in a fall-back transition, prefer 'after'." "Before" for a gap is the pre-transition offset (shift forward); "after" for an overlap is the post-transition offset (the later instant). The same comment explains why PostgreSQL moved away from "prefer standard time": zones such as `Europe/Moscow` (Oct 2014) and `Europe/Dublin` make "standard" ill-defined, so the rule is better phrased as before/after the transition.
- **DuckDB** converts naive timestamps through an ICU `Calendar` in [`ICUFromNaiveTimestamp::Operation` (`extension/icu/icu-timezone.cpp`, v1.5.2)](https://github.com/duckdb/duckdb/blob/v1.5.2/extension/icu/icu-timezone.cpp#L107-L145), created with [`icu::Calendar::createInstance`](https://github.com/duckdb/duckdb/blob/v1.5.2/extension/icu/icu-datefunc.cpp#L47) and never setting a wall-time option, so ICU's defaults apply.
- **ICU** documents those defaults in `calendar.h`: [`setRepeatedWallTimeOption`](https://github.com/unicode-org/icu/blob/1d4445fb29db22a1606fb76b37869837fe01e9a3/icu4c/source/i18n/unicode/calendar.h#L903-L924) ("1:30 AM ... will be interpreted as 1:30 AM EST (last occurrence). The default value is `UCAL_WALLTIME_LAST`") and [`setSkippedWallTimeOption`](https://github.com/unicode-org/icu/blob/1d4445fb29db22a1606fb76b37869837fe01e9a3/icu4c/source/i18n/unicode/calendar.h#L936-L958) ("2:30 AM is interpreted as 31 minutes after 1:59 AM EST, therefore, it will be resolved as 3:30 AM EDT ... The default value is `UCAL_WALLTIME_LAST`").

Not every ecosystem agrees on the overlap case: Java's `ZonedDateTime.of(LocalDateTime, ZoneId)` and Python's `zoneinfo` (`fold=0`) pick the *earlier* occurrence. The SQL engines above pick the later one.

**Additional context**

- Reported against DataFusion as https://github.com/apache/datafusion/issues/25084. I first prototyped a DataFusion-side workaround (https://github.com/apache/datafusion/pull/25115, now closed) but the kernel is the right place: every arrow-rs consumer hits this, and DataFusion has cast paths that call the kernel directly.
- PR: https://github.com/apache/arrow-rs/pull/11038 changes `adjust_timestamp_to_timezone` to the resolution above. Adding an explicit policy to `CastOptions` (like Arrow C++'s `AssumeTimezoneOptions` with `ambiguous`/`nonexistent` = raise/earliest/latest) would be a breaking change to a public struct, so I am proposing the deterministic behaviour as the fix now; a policy option can follow in a major release if anyone needs `earliest` or `raise`.
- `string_to_datetime` in `arrow-cast/src/parse.rs` has the same `.single()` pattern in three places (`Error parsing timestamp ...: error computing timezone offset`). That is a separate, generic-`TimeZone` code path, tracked in #11039.

Contributor guide

Open the contributing guide

Research direction

Start in arrow-cast/src/cast/mod.rs at adjust_timestamp_to_timezone and reproduce the DST cases from the issue with the shown cast options. Verify that ambiguous and nonexistent named-timezone readings resolve to the stated instants, while unambiguous and fixed-offset cases remain unchanged; string_to_datetime in arrow-cast/src/parse.rs is explicitly separate work tracked in #11039.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.