This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11054-274f433115806af8a5ba557131f1c3fb9fc91630 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 89fc173a933d78e5cee08a47c901209c2de59521 Author: Adrian Garcia Badaracco <[email protected]> AuthorDate: Sun Nov 1 06:30:42 2026 +0000 Resolve DST-ambiguous and nonexistent local times in `string_to_datetime` (#11054) # Which issue does this PR close? - Closes https://github.com/apache/arrow-rs/issues/11039 and https://github.com/apache/datafusion/issues/25084 # Rationale for this change ## What a user hits today Parse a timestamp string into a named timezone. Pick a wall clock reading that the zone skips or repeats. The parse fails. ```rust use arrow_array::timezone::Tz; use arrow_cast::parse::string_to_datetime; let tz: Tz = "America/New_York".parse().unwrap(); // 02:30 does not exist on 2024-03-10: the clocks go forward at 02:00. string_to_datetime(&tz, "2024-03-10 02:30:00"); // 01:30 happens twice on 2024-11-03: the clocks go back at 02:00. string_to_datetime(&tz, "2024-11-03 01:30:00"); ``` Today: ```text gap -> Err("Parser error: Error parsing timestamp from '2024-03-10 02:30:00': error computing timezone offset") ambiguous -> Err("Parser error: Error parsing timestamp from '2024-11-03 01:30:00': error computing timezone offset") ``` After this PR: ```text gap -> Ok("2024-03-10T07:30:00+00:00") ambiguous -> Ok("2024-11-03T06:30:00+00:00") ``` Every `Utf8` / `LargeUtf8` / `Utf8View` cast to `Timestamp(_, Some(tz))` goes through that function. Under `CastOptions { safe: true }` the error becomes a **silent NULL** for the whole row. ## How this reaches a SQL user A DataFusion user reaches the parser through ordinary SQL. #11038 fixed the cast kernel, so the first line below now works and the other two still fail: ```sql SELECT '2024-03-10 02:30:00'::timestamp AT TIME ZONE 'America/New_York'; -- cast kernel: fixed by #11038 SELECT '2024-03-10 02:30:00' AT TIME ZONE 'America/New_York'; -- parser: this PR SET datafusion.execution.time_zone = 'America/New_York'; SELECT '2024-03-10 02:30:00'::timestamptz; -- parser: this PR ``` To a user those are the same query, and the last spelling is the most natural one. With only #11038 merged, an explicit `::timestamp` in the middle of an expression makes a query start to work. That is harder to explain than the current uniform failure. So this PR completes #11038, it is not an optional follow-up. ## Why the parser fails `string_to_datetime` in `arrow-cast/src/parse.rs` resolves a wall clock reading with `LocalResult::single()`. That is `None` in two distinct cases: * the reading is **ambiguous** — the repeated hour when the clocks go back; * the reading is **nonexistent** — the skipped hour when the clocks go forward. There are three such call sites, one per shape of input: | # | Site | Example input | | - | ---- | ------------- | | 1 | date only, reads local midnight | `'2018-11-04'` | | 2 | naive datetime, no trailing offset or zone | `'2024-03-10 02:30:00'` | | 3 | datetime with a trailing IANA zone name | `'2024-03-10 02:30:00 America/New_York'` | # What changes are included in this PR? ## Overview One shared function now decides every wall clock reading in arrow-cast. The three parser sites and the cast kernel all call it. The policy itself does not change. ## Detail The resolution policy is deliberately shared with #11038, not restated. Two copies of this policy that drift apart is exactly the bug this PR avoids. One such divergence already exists in the tree, and this PR leaves it alone. See the follow-up section below. So #11038's `resolve_local_offset` moves out of `cast/temporal.rs` into a new private `arrow-cast/src/local_time.rs` module and becomes `pub(crate)`. It generalises from `&Tz` to any `chrono::TimeZone`, because `string_to_datetime` is generic over its target timezone. A thin `resolve_local_datetime` wrapper returns a `DateTime<T>`, which is the shape the three parser sites want. All four call sites now go through the one function. The policy is unchanged from #11038: * An **ambiguous** reading takes the **later** instant. This is the offset in effect after the transition. * A **nonexistent** reading moves **forward** by the length of the gap. The code recovers the pre-transition offset with one probe 24 hours earlier, and takes the earliest result. Two properties of the timezone database make the probe safe: * No two transitions in any of the 597 zones are closer than **167 hours**. So the probe lands on the correct side of the transition. * No local gap is longer than **24 hours**. So the probe lands outside the gap at all. Both were verified exhaustively for #11038. This PR does not re-derive them. ## On site 3, the zone named in the string Site 3 resolves against a zone the user spelled out, rather than one taken from the target type. So it is fair to ask whether it deserves stricter treatment. It gets the **same** policy, for two reasons: 1. A named zone makes the *zone* unambiguous. It does nothing to make the *wall clock reading* unambiguous. `'2024-03-10 02:30:00 America/New_York'` is exactly as unresolvable as `'2024-03-10 02:30:00'` read in `America/New_York`, and the user has no other spelling for what they meant. 2. PostgreSQL 17 makes no distinction between the two spellings. `SET TimeZone='America/New_York'; SELECT '2024-03-10 02:30:00'::timestamptz;` and `SELECT timestamptz '2024-03-10 02:30:00 America/New_York';` both return `2024-03-10 07:30:00+00`. This was measured, not assumed. A different rule for site 3 would put the "same query, different spelling, different outcome" split back inside a single function. That split is what this PR exists to remove. # Are these changes tested? Yes. **Every expected instant in the new tests came from a real PostgreSQL 17.11** (`postgres:17`). None were derived by hand. All three call sites are covered for both an ambiguous and a nonexistent reading: * `America/New_York` and `America/Los_Angeles` — a whole-hour DST step. * `Australia/Lord_Howe` — a **thirty minute** step, so a gap shifts `02:15` to `02:45`, not `03:15`. This rules out any implementation that assumes a one-hour gap. * `Pacific/Chatham` — `+12:45` / `+13:45`, an offset that is not a whole number of hours on either side. * `Australia/Sydney` — southern hemisphere, so the gap falls in October and the repeated hour in April. This catches implementations that reach for `offset_from_utc_datetime` as a shortcut, which returns the post-transition offset and is wrong here. * The date-only site, through the two zones whose transitions land on midnight and are therefore unreachable from any datetime input: * `America/Sao_Paulo` `2018-11-04` — local midnight **does not exist**, because Brazil started DST at 00:00. Result: `2018-11-04T03:00:00Z`. * `America/Havana` `2024-11-03` — local midnight **happens twice**, because Cuba ends DST at 00:00. Result: the later instant, `2024-11-03T05:00:00Z`. * A fixed offset (`+05:30`), which has no transitions and must be unaffected. * `CastOptions { safe: true }` against `safe: false` at the array level. The former produced a silent NULL and the latter an error. Both now produce the resolved instant, and genuinely unparseable input still nulls or errors as before. * An array that straddles both of a zone's 2024 transitions, across `StringArray`, `LargeStringArray` and `StringViewArray`. This confirms the offset resolves **per row**, not once for the array. * Sub-second precision survives the shift applied to a nonexistent reading. Counterfactually validated. Revert the three call sites and keep the tests, and 9 of the new tests fail. The `safe: true` case fails with `left: 0`, which is the silent-NULL mode. ``` cargo test -p arrow-cast # and --all-features, --doc, --doc --no-default-features cargo clippy -p arrow-cast --all-targets --all-features -- -D warnings cargo test -p arrow --features chrono-tz cargo test -p arrow-csv -p arrow-json -p arrow-array cargo fmt --all ``` `arrow-cast` already gained `arrow-array/chrono-tz` as a dev-dependency in #11038, so no manifest change is needed here. # Field research: PostgreSQL 17 and DuckDB The resolution policy is the one arbitrary decision in this stack, 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. | Case | PostgreSQL 17.11 | DuckDB 1.5.2 | This PR | | --- | --- | --- | --- | | `America/New_York` `2024-11-03 01:30` — **ambiguous** | `2024-11-03 06:30:00` | `2024-11-03 06:30:00` | `2024-11-03 06:30:00` | | `America/Havana` `2024-11-03 00:00` — **ambiguous** local midnight | `2024-11-03 05:00:00` | `2024-11-03 05:00:00` | `2024-11-03 05:00:00` | | `America/New_York` `2024-03-10 02:30` — gap | `2024-03-10 07:30:00` | `2024-03-10 07:30:00` | `2024-03-10 07:30:00` | | `America/Sao_Paulo` `2018-11-04 00:00` — local midnight does not exist | `2018-11-04 03:00:00` | `2018-11-04 03:00:00` | `2018-11-04 03:00:00` | | `Australia/Sydney` `2024-10-06 02:30` — southern hemisphere gap | `2024-10-05 16:30:00` | `2024-10-05 16:30:00` | `2024-10-05 16:30:00` | | `Australia/Lord_Howe` `2024-10-06 02:15` — 30 minute DST step | `2024-10-05 15:45:00` | `2024-10-05 15:45:00` | `2024-10-05 15:45:00` | | `Pacific/Chatham` `2024-09-29 03:00` — +12:45 / +13:45 | `2024-09-28 14:15:00` | `2024-09-28 14:15:00` | `2024-09-28 14:15:00` | **The 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:30` in New York is either `05:30Z` (EDT) or `06:30Z` (EST). Both engines give `06:30Z`. Havana's ambiguous midnight is either `04:00Z` (CDT) or `05:00Z` (CST). Both engines give `05: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: | Spelling | PostgreSQL 17.11 | Convention | | --- | --- | --- | | `SET TimeZone='+05:30'` then `'2024-01-01 12:00:00'::timestamptz` | `17:30Z` | POSIX, west-positive | | `'2024-01-01 12:00:00'::timestamp AT TIME ZONE '+05:30'` | `17:30Z` | POSIX, west-positive | | `timestamptz '2024-01-01 12:00:00 +05:30'` | `06:30Z` | ISO, east-positive | PostgreSQL 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 https://github.com/apache/datafusion/issues/25170. # Are there any user-facing changes? Yes, and the change is intentional. A string that previously failed to parse, or silently became NULL, now parses to the instant PostgreSQL and DuckDB produce. There is no API change. `resolve_local_offset` stays crate-private. ## Together with #11038 this closes DataFusion #25084 Neither PR closes it alone. #11038 covers the cast kernel. This PR covers the parser. A user hits both through the same SQL, so both must land. ## One existing test changes expectation `arrow/tests/timezone.rs` asserted the old error for two `America/Los_Angeles` cases. They move from `test_parse_timezone_invalid` to `test_parse_timezone`: | Input | Before | After (= PostgreSQL 17) | | ----- | ------ | ----------------------- | | `2023-03-12 02:05:06 America/Los_Angeles` | `error computing timezone offset` | `2023-03-12T10:05:06+00:00` | | `2023-11-05 01:30:06 America/Los_Angeles` | `error computing timezone offset` | `2023-11-05T09:30:06+00:00` | ## Downstream DataFusion has one matching expectation, at `datafusion/sqllogictest/test_files/datetime/timestamps.slt:2360-2366`. It asserts that `SELECT TIMESTAMPTZ '2023-03-12 02:00:00 America/Los_Angeles'` errors, and its own comment already notes `# postgresql: accepts`. On the next arrow upgrade it becomes a `query P` that returns `2023-03-12T10:00:00Z`, which is what PostgreSQL returns. This PR includes no DataFusion change. ## Follow-up: `arrow-array` resolves ambiguity the opposite way This PR creates one shared helper for arrow-cast, but one divergence remains in the tree. It is deliberately untouched here: | | Ambiguous | In a gap | | --- | --- | --- | | `arrow-cast::local_time::resolve_local_offset` | the **later** instant | offset probed 24 hours earlier | | `arrow-array::types::from_naive_datetime` ([`types.rs:347`](https://github.com/apache/arrow-rs/blob/main/arrow-array/src/types.rs#L347)) | the **earlier** instant (`Ambiguous(dt1, _)`) | `None` | Measured in `America/New_York` for `2024-11-03 01:30:00`: `from_naive_datetime` gives `05:30Z`, and this stack gives `06:30Z`. This matters downstream. DataFusion calls the `arrow-array` form directly with a `Some(tz)` in `datafusion/functions/src/datetime/date_part.rs` (`date_to_scalar`, four call sites). Where the local midnight it builds is ambiguous, the earlier instant becomes an **upper** bound, so valid rows can drop out. The behaviour predates this stack. Changing `from_naive_datetime` affects every caller of `arrow-array`, so it deserves its own review rather than a ride along with a parser fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <[email protected]> --- arrow-cast/src/cast/mod.rs | 153 ++++++++++++++++++++++ arrow-cast/src/cast/temporal.rs | 65 +--------- arrow-cast/src/lib.rs | 2 + arrow-cast/src/local_time.rs | 106 ++++++++++++++++ arrow-cast/src/parse.rs | 272 ++++++++++++++++++++++++++++++++++++++-- arrow-csv/Cargo.toml | 3 + arrow-csv/src/reader/mod.rs | 42 +++++++ arrow-json/Cargo.toml | 3 + arrow-json/src/reader/mod.rs | 30 +++++ arrow/tests/timezone.rs | 28 +++-- 10 files changed, 620 insertions(+), 84 deletions(-) diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs index 760e43d0e0..d24d106c5e 100644 --- a/arrow-cast/src/cast/mod.rs +++ b/arrow-cast/src/cast/mod.rs @@ -3983,6 +3983,159 @@ mod tests { assert!(c.is_null(3)); } + /// The rows of the string array used by the string -> timestamp DST tests, + /// and the instants they denote in `America/New_York` according to + /// PostgreSQL 17.11. + /// + /// They deliberately straddle both transitions of 2024, so a single offset + /// resolved once for the whole array cannot satisfy all of them. + const DST_STRINGS: [&str; 5] = [ + // EDT (-04:00), before the November transition. + "2024-11-02 12:00:00", + // Ambiguous: 01:30 happens twice, the later (EST) instant wins. + "2024-11-03 01:30:00", + // EST (-05:00), after the November transition. + "2024-11-03 12:00:00", + // Nonexistent: 02:30 is skipped in March and shifts to 03:30 EDT. + "2024-03-10 02:30:00", + // EST (-05:00), before the March transition. + "2024-03-09 12:00:00", + ]; + + const DST_SECONDS: [i64; 5] = [ + 1_730_563_200, + 1_730_615_400, + 1_730_653_200, + 1_710_055_800, + 1_710_003_600, + ]; + + // Casting strings to `Timestamp(_, Some(tz))` goes through + // `string_to_datetime`, not through `adjust_timestamp_to_timezone`, so it + // needs its own coverage of the DST rules. See `crate::local_time`. + #[test] + fn test_cast_string_to_named_timezone_dst() { + let to_type = DataType::Timestamp(TimeUnit::Second, Some("America/New_York".into())); + let options = CastOptions { + safe: false, + ..Default::default() + }; + + let mut values: Vec<Option<&str>> = DST_STRINGS.iter().copied().map(Some).collect(); + values.push(None); + + let arrays: Vec<ArrayRef> = vec![ + Arc::new(StringArray::from(values.clone())), + Arc::new(LargeStringArray::from(values.clone())), + Arc::new(StringViewArray::from(values)), + ]; + + for array in arrays { + let from_type = array.data_type().clone(); + let b = cast_with_options(&array, &to_type, &options) + .unwrap_or_else(|e| panic!("{from_type:?}: {e}")); + assert_eq!(b.data_type(), &to_type); + let c = b.as_primitive::<TimestampSecondType>(); + for (i, expected) in DST_SECONDS.iter().enumerate() { + assert_eq!(c.value(i), *expected, "{from_type:?} row {i}"); + } + assert!(c.is_null(5), "{from_type:?} null row"); + } + } + + // The same values must not silently become null under `safe` casting either. + #[test] + fn test_cast_string_to_named_timezone_dst_safe() { + let to_type = DataType::Timestamp(TimeUnit::Second, Some("America/New_York".into())); + let options = CastOptions { + safe: true, + ..Default::default() + }; + + let array = StringArray::from(vec![ + Some(DST_STRINGS[1]), + Some(DST_STRINGS[3]), + Some("not a timestamp"), + None, + ]); + + let b = cast_with_options(&array, &to_type, &options).unwrap(); + let c = b.as_primitive::<TimestampSecondType>(); + assert_eq!(c.value(0), DST_SECONDS[1]); + assert_eq!(c.value(1), DST_SECONDS[3]); + // Genuinely unparsable input still becomes null. + assert!(c.is_null(2)); + assert!(c.is_null(3)); + assert_eq!(c.null_count(), 2); + } + + // Sub-second precision survives the shift applied to a nonexistent reading. + #[test] + fn test_cast_string_to_named_timezone_dst_nanos() { + let to_type = DataType::Timestamp(TimeUnit::Nanosecond, Some("America/New_York".into())); + let options = CastOptions { + safe: false, + ..Default::default() + }; + + let array = StringArray::from(vec![ + "2024-03-10 02:30:00.123456789", + "2024-11-03 01:30:00.123456789", + ]); + let b = cast_with_options(&array, &to_type, &options).unwrap(); + let c = b.as_primitive::<TimestampNanosecondType>(); + assert_eq!(c.value(0), 1_710_055_800 * 1_000_000_000 + 123_456_789); + assert_eq!(c.value(1), 1_730_615_400 * 1_000_000_000 + 123_456_789); + } + + // A date-only string reads local midnight, which is itself skipped or + // repeated in some zones. + #[test] + fn test_cast_string_to_named_timezone_dst_date_only() { + let options = CastOptions { + safe: false, + ..Default::default() + }; + + // Local midnight does not exist in Sao Paulo on 2018-11-04. + let to_type = DataType::Timestamp(TimeUnit::Second, Some("America/Sao_Paulo".into())); + let array = StringArray::from(vec!["2018-11-04"]); + let b = cast_with_options(&array, &to_type, &options).unwrap(); + assert_eq!( + b.as_primitive::<TimestampSecondType>().value(0), + 1_541_300_400 + ); + + // Local midnight happens twice in Havana on 2024-11-03. + let to_type = DataType::Timestamp(TimeUnit::Second, Some("America/Havana".into())); + let array = StringArray::from(vec!["2024-11-03"]); + let b = cast_with_options(&array, &to_type, &options).unwrap(); + assert_eq!( + b.as_primitive::<TimestampSecondType>().value(0), + 1_730_610_000 + ); + } + + // A timezone named by the string itself is resolved the same way, matching + // PostgreSQL. + #[test] + fn test_cast_string_to_timezone_named_in_string_dst() { + let to_type = DataType::Timestamp(TimeUnit::Second, Some("UTC".into())); + let options = CastOptions { + safe: false, + ..Default::default() + }; + + let array = StringArray::from(vec![ + "2024-03-10 02:30:00 America/New_York", + "2024-11-03 01:30:00 America/New_York", + ]); + let b = cast_with_options(&array, &to_type, &options).unwrap(); + let c = b.as_primitive::<TimestampSecondType>(); + assert_eq!(c.value(0), 1_710_055_800); + assert_eq!(c.value(1), 1_730_615_400); + } + // The same values must not become null when `safe` casting is requested. #[test] fn test_cast_timestamp_to_named_timezone_dst_safe() { diff --git a/arrow-cast/src/cast/temporal.rs b/arrow-cast/src/cast/temporal.rs index 66a3351229..114287a1e3 100644 --- a/arrow-cast/src/cast/temporal.rs +++ b/arrow-cast/src/cast/temporal.rs @@ -21,11 +21,13 @@ use arrow_array::{cast::*, temporal_conversions::*, timezone::Tz, types::*, *}; use arrow_buffer::IntervalMonthDayNano; use arrow_schema::{ArrowError, DataType, TimeUnit}; -use chrono::{FixedOffset, LocalResult, NaiveDateTime, NaiveTime, Offset, TimeDelta, TimeZone}; +use chrono::{NaiveTime, Offset}; use std::sync::Arc; use num_traits::AsPrimitive; +use crate::local_time::resolve_local_offset; + use super::{CastOptions, time_unit_multiple, units_per_day}; /// Cast the array from interval year month to month day nano @@ -404,65 +406,6 @@ pub(crate) fn timestamp_to_date64<T: ArrowTimestampType>( Ok(Arc::new(array)) } -/// Returns the offset to use when interpreting `local` as a wall clock reading -/// in `tz`, or `None` if it cannot be resolved. -/// -/// `None` is not expected in practice. With the current timezone database no -/// reading reaches it, because every ambiguous or nonexistent reading resolves -/// as described below. The `None` path is a safeguard against a future -/// timezone database that breaks the assumptions of the gap handling. Callers -/// then apply their usual error or null handling. -/// -/// In an IANA timezone a wall clock reading does not always identify a unique -/// instant, and this function picks one following the same rules as PostgreSQL -/// and DuckDB: -/// -/// * **Ambiguous** -- when the clocks go back ("fall back") the same reading -/// occurs twice. The *later* instant is chosen, i.e. the offset in effect -/// after the transition. For example `2024-11-03T01:30:00` in -/// `America/New_York` is read as `-05:00` (EST), not `-04:00` (EDT). -/// * **Nonexistent** -- when the clocks go forward ("spring forward") the -/// reading never occurs. It is shifted forward by the length of the gap, -/// which is the same as reading it with the offset in effect *before* the -/// transition. For example `2024-03-10T02:30:00` in `America/New_York` is -/// read as `-05:00` (EST) and therefore denotes `2024-03-10T03:30:00-04:00`. -/// -/// Timezones with a fixed offset are never ambiguous and have no gaps. -/// -/// See <https://github.com/apache/arrow-rs/issues/11037> for the PostgreSQL and -/// ICU (DuckDB) sources these rules are taken from. -fn resolve_local_offset(tz: &Tz, local: &NaiveDateTime) -> Option<FixedOffset> { - match tz.offset_from_local_datetime(local) { - LocalResult::Single(offset) => Some(offset.fix()), - LocalResult::Ambiguous(_earlier, later) => Some(later.fix()), - LocalResult::None => { - // The reading falls in a gap. Recover the offset in effect before - // the transition by probing 24 hours earlier. - // - // Two separate properties of the timezone database make this sound: - // - // 1. No local gap is longer than 24 hours, so the probe lands - // outside this gap and is itself resolvable. 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 true margin here is one second, not a - // comfortable one. - // 2. No two transitions are closer together than 24 hours, so the - // offset the probe finds is the one in effect immediately before - // this transition, and not some older offset. The smallest - // observed interval is 167 hours (`America/Boa_Vista`, 2000). - // - // Property 1 is what makes the probe resolvable; property 2 is what - // makes the answer correct. If the probe is still unresolvable, give - // up and let the caller apply the usual error / null handling. - tz.offset_from_local_datetime(&(*local - TimeDelta::hours(24))) - .earliest() - .map(|offset| offset.fix()) - } - } -} - pub(crate) fn adjust_timestamp_to_timezone<T: ArrowTimestampType>( array: PrimitiveArray<Int64Type>, to_tz: &Tz, @@ -470,7 +413,7 @@ pub(crate) fn adjust_timestamp_to_timezone<T: ArrowTimestampType>( ) -> Result<PrimitiveArray<Int64Type>, ArrowError> { let adjust = |o| { let local = as_datetime::<T>(o)?; - let offset = resolve_local_offset(to_tz, &local)?; + let offset = resolve_local_offset(to_tz, &local)?.fix(); T::from_naive_datetime(local - offset, None) }; let adjusted = if cast_options.safe { diff --git a/arrow-cast/src/lib.rs b/arrow-cast/src/lib.rs index 4cd785809a..c13745c334 100644 --- a/arrow-cast/src/lib.rs +++ b/arrow-cast/src/lib.rs @@ -38,3 +38,5 @@ pub mod parse; pub mod pretty; pub mod base64; + +mod local_time; diff --git a/arrow-cast/src/local_time.rs b/arrow-cast/src/local_time.rs new file mode 100644 index 0000000000..e74ba63f36 --- /dev/null +++ b/arrow-cast/src/local_time.rs @@ -0,0 +1,106 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Resolving wall clock ("local") readings to instants. +//! +//! Reading a [`NaiveDateTime`] as a wall clock time in an IANA timezone does not +//! always identify a unique instant, because of daylight savings transitions. +//! Everything that has to make that choice -- the cast kernel and the string +//! parser -- goes through [`resolve_local_offset`] so that the two cannot drift +//! apart. + +use chrono::{DateTime, LocalResult, NaiveDateTime, Offset, TimeDelta, TimeZone}; + +/// Returns the offset to use when interpreting `local` as a wall clock reading +/// in `tz`, or `None` if it cannot be resolved. +/// +/// In an IANA timezone a wall clock reading does not always identify a unique +/// instant, and this function picks one following the same rules as PostgreSQL +/// and DuckDB: +/// +/// * **Ambiguous** -- when the clocks go back ("fall back") the same reading +/// occurs twice. The *later* instant is chosen, i.e. the offset in effect +/// after the transition. For example `2024-11-03T01:30:00` in +/// `America/New_York` is read as `-05:00` (EST), not `-04:00` (EDT). +/// * **Nonexistent** -- when the clocks go forward ("spring forward") the +/// reading never occurs. It is shifted forward by the length of the gap, +/// which is the same as reading it with the offset in effect *before* the +/// transition. For example `2024-03-10T02:30:00` in `America/New_York` is +/// read as `-05:00` (EST) and therefore denotes `2024-03-10T03:30:00-04:00`. +/// +/// Timezones with a fixed offset are never ambiguous and have no gaps. +/// +/// The gap rule probes the offset 24 hours earlier. This is correct for the +/// IANA timezone database, but this function accepts any [`TimeZone`]. For a +/// timezone whose transitions do not obey the assumptions given in the body +/// (no gap longer than 24 hours, no two transitions closer than 24 hours), the +/// result can be `None` or an incorrect offset. +/// +/// `None` is therefore not expected in practice: with the current timezone +/// database every ambiguous or nonexistent reading resolves as described +/// above, and the `None` path is only a safeguard. Callers then apply their +/// usual error or null handling. +/// +/// See <https://github.com/apache/arrow-rs/issues/11037> for the PostgreSQL and +/// ICU (DuckDB) sources these rules are taken from. +pub(crate) fn resolve_local_offset<T: TimeZone>( + tz: &T, + local: &NaiveDateTime, +) -> Option<T::Offset> { + match tz.offset_from_local_datetime(local) { + LocalResult::Single(offset) => Some(offset), + LocalResult::Ambiguous(_earlier, later) => Some(later), + LocalResult::None => { + // The reading falls in a gap. Recover the offset in effect before + // the transition by probing 24 hours earlier. + // + // Two separate properties of the timezone database make this sound: + // + // 1. No local gap is longer than 24 hours, so the probe lands + // outside this gap and is itself resolvable. 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 true margin here is one second, not a + // comfortable one. + // 2. No two transitions are closer together than 24 hours, so the + // offset the probe finds is the one in effect immediately before + // this transition, and not some older offset. The smallest + // observed interval is 167 hours (`America/Boa_Vista`, 2000). + // + // Property 1 is what makes the probe resolvable; property 2 is what + // makes the answer correct. If the probe is still unresolvable, give + // up and let the caller apply the usual error / null handling. + tz.offset_from_local_datetime(&(*local - TimeDelta::hours(24))) + .earliest() + } + } +} + +/// Reads `local` as a wall clock time in `tz`, resolving daylight savings +/// ambiguities and gaps as described on [`resolve_local_offset`]. +/// +/// This is [`TimeZone::from_local_datetime`] with the two unresolvable +/// [`LocalResult`] cases decided rather than rejected. +pub(crate) fn resolve_local_datetime<T: TimeZone>( + tz: &T, + local: &NaiveDateTime, +) -> Option<DateTime<T>> { + let offset = resolve_local_offset(tz, local)?; + let utc = local.checked_sub_offset(offset.fix())?; + Some(DateTime::from_naive_utc_and_offset(utc, offset)) +} diff --git a/arrow-cast/src/parse.rs b/arrow-cast/src/parse.rs index 78b1d7dc36..ab511a1d13 100644 --- a/arrow-cast/src/parse.rs +++ b/arrow-cast/src/parse.rs @@ -18,6 +18,7 @@ //! [`Parser`] implementations for converting strings to Arrow types //! //! Used by the CSV and JSON readers to convert strings to Arrow types +use crate::local_time::resolve_local_datetime; use arrow_array::ArrowNativeTypeOp; use arrow_array::timezone::Tz; use arrow_array::types::*; @@ -174,8 +175,24 @@ impl TimestampParser { /// /// * `2023-01-01 040506 America/Los_Angeles` /// -/// If a timestamp is ambiguous, for example as a result of daylight-savings time, an error -/// will be returned +/// A timestamp with no explicit offset is read as a wall clock time in the +/// relevant timezone, which for an IANA timezone does not always identify a +/// unique instant. Daylight savings transitions are resolved the same way +/// PostgreSQL and DuckDB resolve them: an *ambiguous* reading (the repeated +/// hour when the clocks go back) takes the later instant, and a *nonexistent* +/// reading (the skipped hour when the clocks go forward) is shifted forward by +/// the length of the gap. +/// +/// ``` +/// # use arrow_cast::parse::string_to_datetime; +/// # use chrono::Utc; +/// // 02:30 does not exist in America/New_York on 2024-03-10; it is read as 03:30 EDT +/// let gap = string_to_datetime(&Utc, "2024-03-10 02:30:00 America/New_York").unwrap(); +/// assert_eq!(gap.to_rfc3339(), "2024-03-10T07:30:00+00:00"); +/// // 01:30 happens twice on 2024-11-03; the later (EST) instant is used +/// let ambiguous = string_to_datetime(&Utc, "2024-11-03 01:30:00 America/New_York").unwrap(); +/// assert_eq!(ambiguous.to_rfc3339(), "2024-11-03T06:30:00+00:00"); +/// ``` /// /// Some formats supported by PostgresSql <https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-TIME-TABLE> /// are not supported, like @@ -198,9 +215,7 @@ pub fn string_to_datetime<T: TimeZone>(timezone: &T, s: &str) -> Result<DateTime let date = parser.date().ok_or_else(|| err("error parsing date"))?; if bytes.len() == 10 { let datetime = date.and_time(NaiveTime::MIN); - return timezone - .from_local_datetime(&datetime) - .single() + return resolve_local_datetime(timezone, &datetime) .ok_or_else(|| err("error computing timezone offset")); } @@ -219,9 +234,7 @@ pub fn string_to_datetime<T: TimeZone>(timezone: &T, s: &str) -> Result<DateTime } if bytes.len() <= tz_offset { - return timezone - .from_local_datetime(&datetime) - .single() + return resolve_local_datetime(timezone, &datetime) .ok_or_else(|| err("error computing timezone offset")); } @@ -231,9 +244,7 @@ pub fn string_to_datetime<T: TimeZone>(timezone: &T, s: &str) -> Result<DateTime // Parse remainder of string as timezone let parsed_tz: Tz = s[tz_offset..].trim_start().parse()?; - let parsed = parsed_tz - .from_local_datetime(&datetime) - .single() + let parsed = resolve_local_datetime(&parsed_tz, &datetime) .ok_or_else(|| err("error computing timezone offset"))?; Ok(parsed.with_timezone(timezone)) @@ -3850,4 +3861,243 @@ mod tests { Time32SecondType::parse(time) ); } + + /// Parses `s` as a wall clock reading in `zone` and returns the resulting + /// instant, as (seconds since the epoch, RFC3339 rendering in UTC). + /// + /// `zone` plays the role of the target `Timestamp(_, Some(tz))` timezone. + fn parse_in_zone(zone: &str, s: &str) -> (i64, String) { + let tz: Tz = zone.parse().unwrap(); + let dt = string_to_datetime(&tz, s).unwrap(); + (dt.timestamp(), dt.to_utc().to_rfc3339()) + } + + /// Parses `s`, which names its own timezone, against a UTC target. + fn parse_with_named_zone(s: &str) -> (i64, String) { + let dt = string_to_datetime(&Utc, s).unwrap(); + (dt.timestamp(), dt.to_utc().to_rfc3339()) + } + + // A naive datetime read as a wall clock time in the target timezone. This is + // the `'2024-03-10 02:30:00'::timestamptz` spelling, where the timezone comes + // from the target type rather than from the string. + // + // Every expectation here was taken from PostgreSQL 17.11 via + // `SET TimeZone = <zone>; SELECT '<input>'::timestamptz;`. + #[test] + fn test_string_to_datetime_dst_target_timezone() { + // (zone, input, epoch seconds, UTC rendering) + let cases = [ + // America/New_York: a one hour DST step. + // Control: nothing special about this reading. + ( + "America/New_York", + "2024-11-01 00:00:00", + 1_730_433_600, + "2024-11-01T04:00:00+00:00", + ), + // Ambiguous: the clocks go back at 2024-11-03T02:00 EDT, so 01:30 + // happens twice. The later (EST, -05:00) instant wins. + ( + "America/New_York", + "2024-11-03 01:30:00", + 1_730_615_400, + "2024-11-03T06:30:00+00:00", + ), + // Nonexistent: the clocks go forward at 2024-03-10T02:00 EST, so + // 02:30 never happens. It shifts forward to 03:30 EDT. + ( + "America/New_York", + "2024-03-10 02:30:00", + 1_710_055_800, + "2024-03-10T07:30:00+00:00", + ), + // Australia/Sydney: southern hemisphere, so the gap is in October and + // the repeated hour is in April. + ( + "Australia/Sydney", + "2024-04-07 02:30:00", + 1_712_421_000, + "2024-04-06T16:30:00+00:00", + ), + ( + "Australia/Sydney", + "2024-10-06 02:30:00", + 1_728_145_800, + "2024-10-05T16:30:00+00:00", + ), + // Australia/Lord_Howe: a *thirty minute* DST step, so the gap is half + // an hour wide and 02:15 shifts to 02:45 rather than to 03:15. + ( + "Australia/Lord_Howe", + "2024-04-07 01:45:00", + 1_712_416_500, + "2024-04-06T15:15:00+00:00", + ), + ( + "Australia/Lord_Howe", + "2024-10-06 02:15:00", + 1_728_143_100, + "2024-10-05T15:45:00+00:00", + ), + // Pacific/Chatham: +12:45 / +13:45, i.e. an offset that is not a whole + // number of hours on either side of the transition. + ( + "Pacific/Chatham", + "2024-04-07 03:00:00", + 1_712_412_900, + "2024-04-06T14:15:00+00:00", + ), + ( + "Pacific/Chatham", + "2024-09-29 03:00:00", + 1_727_532_900, + "2024-09-28T14:15:00+00:00", + ), + // A fixed offset has no transitions and is unaffected. + ( + "+05:30", + "2024-03-10 02:30:00", + 1_710_018_000, + "2024-03-09T21:00:00+00:00", + ), + ]; + + for (zone, input, epoch, utc) in cases { + assert_eq!( + parse_in_zone(zone, input), + (epoch, utc.to_string()), + "{input} in {zone}" + ); + } + } + + // The date-only branch of `string_to_datetime`, which reads local midnight. + // Midnight is where a transition falls in a number of zones, and it is easy + // to miss when only datetime inputs are considered. + #[test] + fn test_string_to_datetime_dst_date_only() { + let cases = [ + // Brazil started DST at midnight, so 2018-11-04T00:00 does not exist + // in America/Sao_Paulo. It shifts forward to 01:00 -02:00. + ( + "America/Sao_Paulo", + "2018-11-04", + 1_541_300_400, + "2018-11-04T03:00:00+00:00", + ), + // Cuba ends DST at midnight, so 2024-11-03T00:00 happens twice in + // America/Havana. The later (-05:00) instant wins. + ( + "America/Havana", + "2024-11-03", + 1_730_610_000, + "2024-11-03T05:00:00+00:00", + ), + // Control: New York's transition that day is at 02:00, not midnight. + ( + "America/New_York", + "2024-03-10", + 1_710_046_800, + "2024-03-10T05:00:00+00:00", + ), + ]; + + for (zone, input, epoch, utc) in cases { + assert_eq!( + parse_in_zone(zone, input), + (epoch, utc.to_string()), + "{input} in {zone}" + ); + } + } + + // A timezone named by the string itself, e.g. + // `TIMESTAMPTZ '2024-03-10 02:30:00 America/New_York'`. PostgreSQL resolves + // these exactly as it resolves a reading taken against the session timezone, + // and so do we: the zone is named more explicitly, but the wall clock reading + // is no less ambiguous for it. + #[test] + fn test_string_to_datetime_dst_named_zone_in_string() { + let cases = [ + ( + "2024-11-03 01:30:00 America/New_York", + 1_730_615_400, + "2024-11-03T06:30:00+00:00", + ), + ( + "2024-03-10 02:30:00 America/New_York", + 1_710_055_800, + "2024-03-10T07:30:00+00:00", + ), + ( + "2024-04-07 02:30:00 Australia/Sydney", + 1_712_421_000, + "2024-04-06T16:30:00+00:00", + ), + ( + "2024-10-06 02:30:00 Australia/Sydney", + 1_728_145_800, + "2024-10-05T16:30:00+00:00", + ), + ( + "2024-04-07 01:45:00 Australia/Lord_Howe", + 1_712_416_500, + "2024-04-06T15:15:00+00:00", + ), + ( + "2024-10-06 02:15:00 Australia/Lord_Howe", + 1_728_143_100, + "2024-10-05T15:45:00+00:00", + ), + ( + "2024-04-07 03:00:00 Pacific/Chatham", + 1_712_412_900, + "2024-04-06T14:15:00+00:00", + ), + ( + "2024-09-29 03:00:00 Pacific/Chatham", + 1_727_532_900, + "2024-09-28T14:15:00+00:00", + ), + // A fixed offset spelled out in the string stays unaffected. + ( + "2024-03-10 02:30:00 +05:30", + 1_710_018_000, + "2024-03-09T21:00:00+00:00", + ), + ]; + + for (input, epoch, utc) in cases { + assert_eq!( + parse_with_named_zone(input), + (epoch, utc.to_string()), + "{input}" + ); + } + } + + // Sub-second precision survives the shift applied to a nonexistent reading. + #[test] + fn test_string_to_datetime_dst_subsecond() { + let tz: Tz = "America/New_York".parse().unwrap(); + let dt = string_to_datetime(&tz, "2024-03-10 02:30:00.123456789").unwrap(); + assert_eq!(dt.timestamp(), 1_710_055_800); + assert_eq!(dt.timestamp_subsec_nanos(), 123_456_789); + + let dt = + string_to_datetime(&Utc, "2024-11-03 01:30:00.123456789 America/New_York").unwrap(); + assert_eq!(dt.timestamp(), 1_730_615_400); + assert_eq!(dt.timestamp_subsec_nanos(), 123_456_789); + } + + // `string_to_timestamp_nanos` reads against UTC, which has no transitions, so + // none of this changes what it does. + #[test] + fn test_string_to_timestamp_nanos_unaffected_by_dst() { + assert_eq!( + string_to_timestamp_nanos("2024-03-10 02:30:00").unwrap(), + 1_710_037_800 * 1_000_000_000 + ); + } } diff --git a/arrow-csv/Cargo.toml b/arrow-csv/Cargo.toml index 736c1777f4..260a1e1712 100644 --- a/arrow-csv/Cargo.toml +++ b/arrow-csv/Cargo.toml @@ -44,6 +44,9 @@ csv-core = { version = "0.1" } regex = { version = "1.7.0", default-features = false, features = ["std", "unicode", "perf"] } [dev-dependencies] +# Enable `chrono-tz` so that tests can exercise IANA (named) timezones such as +# `America/New_York`, which is where DST transitions can be observed. +arrow-array = { workspace = true, features = ["chrono-tz"] } arrow-buffer = { workspace = true } tempfile = "3.3" futures = "0.3" diff --git a/arrow-csv/src/reader/mod.rs b/arrow-csv/src/reader/mod.rs index 6347b3a3b4..49ef983d64 100644 --- a/arrow-csv/src/reader/mod.rs +++ b/arrow-csv/src/reader/mod.rs @@ -42,6 +42,13 @@ //! let batch = csv.next().unwrap().unwrap(); //! ``` //! +//! # Timestamps +//! +//! Timestamps are parsed with [`arrow_cast::parse::string_to_datetime`]. A +//! timestamp without an explicit offset that falls in a daylight savings gap or +//! overlap of the target timezone is resolved as described there, rather than +//! returning an error. +//! //! # Example: Numeric calculations on CSV //! This code finds the maximum value in column 0 of a CSV file containing //! ```csv @@ -2524,6 +2531,41 @@ mod tests { test_parse_timestamp_impl::<TimestampSecondType>(Some("-03".into()), &[10_800, 0, -7_200]); } + // A local time in a DST gap or overlap of the target timezone is resolved + // as described in `arrow_cast::parse::string_to_datetime`: an ambiguous + // reading takes the later instant, and a nonexistent reading is shifted + // forward by the length of the gap. + #[test] + fn test_parse_timestamp_dst_transitions() { + let csv = [ + // Nonexistent: 02:30 is skipped, read as 03:30 EDT + "2024-03-10 02:30:00", + // Ambiguous: 01:30 happens twice, the later (EST) instant wins + "2024-11-03 01:30:00", + ] + .join("\n"); + let timezone: Option<Arc<str>> = Some("America/New_York".into()); + let schema = Arc::new(Schema::new(vec![Field::new( + "field", + DataType::Timestamp(TimeUnit::Second, timezone.clone()), + true, + )])); + + let mut decoder = ReaderBuilder::new(schema).build_decoder(); + let decoded = decoder.decode(csv.as_bytes()).unwrap(); + assert_eq!(decoded, csv.len()); + decoder.decode(&[]).unwrap(); + + let batch = decoder.flush().unwrap().unwrap(); + let col = batch.column(0).as_primitive::<TimestampSecondType>(); + // 2024-03-10T07:30:00Z and 2024-11-03T06:30:00Z + assert_eq!(col.values(), &[1_710_055_800, 1_730_615_400]); + assert_eq!( + col.data_type(), + &DataType::Timestamp(TimeUnit::Second, timezone) + ); + } + #[test] #[cfg_attr(miri, ignore)] // Takes too long fn test_infer_schema_from_multiple_files() { diff --git a/arrow-json/Cargo.toml b/arrow-json/Cargo.toml index 7a377513a4..12253a684e 100644 --- a/arrow-json/Cargo.toml +++ b/arrow-json/Cargo.toml @@ -54,6 +54,9 @@ ryu = "1.0" itoa = "1.0" [dev-dependencies] +# Enable `chrono-tz` so that tests can exercise IANA (named) timezones such as +# `America/New_York`, which is where DST transitions can be observed. +arrow-array = { workspace = true, features = ["chrono-tz"] } flate2 = { version = "1", default-features = false, features = ["rust_backend"] } serde = { version = "1.0", default-features = false, features = ["derive"] } futures = "0.3" diff --git a/arrow-json/src/reader/mod.rs b/arrow-json/src/reader/mod.rs index 8ac122d925..d8f5027a50 100644 --- a/arrow-json/src/reader/mod.rs +++ b/arrow-json/src/reader/mod.rs @@ -25,6 +25,13 @@ //! parsing of sequences of one or more arbitrarily formatted JSON values, including //! but not limited to newline-delimited JSON. //! +//! # Timestamps +//! +//! Timestamps are parsed with [`arrow_cast::parse::string_to_datetime`]. A +//! timestamp without an explicit offset that falls in a daylight savings gap or +//! overlap of the target timezone is resolved as described there, rather than +//! returning an error. +//! //! # Basic Usage //! //! [`Reader`] can be used directly with synchronous data sources, such as [`std::fs::File`] @@ -2016,6 +2023,29 @@ mod tests { test_timestamp::<TimestampNanosecondType>(); } + // A local time in a DST gap or overlap of the target timezone is resolved + // as described in `arrow_cast::parse::string_to_datetime`: an ambiguous + // reading takes the later instant, and a nonexistent reading is shifted + // forward by the length of the gap. + #[test] + fn test_timestamp_dst_transitions() { + let buf = r#" + {"a": "2024-03-10 02:30:00"} + {"a": "2024-11-03 01:30:00"} + "#; + let data_type = DataType::Timestamp(TimeUnit::Second, Some("America/New_York".into())); + let schema = Arc::new(Schema::new(vec![Field::new("a", data_type, true)])); + + let batches = do_read(buf, 1024, true, false, schema); + assert_eq!(batches.len(), 1); + + let col = batches[0].column(0).as_primitive::<TimestampSecondType>(); + assert_eq!(col.null_count(), 0); + // Nonexistent 02:30 is read as 03:30 EDT (2024-03-10T07:30:00Z), and + // ambiguous 01:30 takes the later EST instant (2024-11-03T06:30:00Z) + assert_eq!(col.values(), &[1_710_055_800, 1_730_615_400]); + } + fn test_time<T: ArrowTemporalType>() { let buf = r#" {"a": 1, "b": "09:26:56.123 AM", "c": 38.30} diff --git a/arrow/tests/timezone.rs b/arrow/tests/timezone.rs index 7b5ec8fbec..61c789a673 100644 --- a/arrow/tests/timezone.rs +++ b/arrow/tests/timezone.rs @@ -41,6 +41,22 @@ fn test_parse_timezone() { "2023-03-12 040506 America/Los_Angeles", "2023-03-12T11:05:06+00:00", ), // Daylight savings + ( + // Sunday, 12 March 2023, 02:00:00 clocks are turned forward 1 hour to + // Sunday, 12 March 2023, 03:00:00 local daylight time instead, so + // 02:05:06 never happens. It is shifted forward by the length of the + // gap, to 03:05:06 PDT, as PostgreSQL and DuckDB do. + "2023-03-12 02:05:06 America/Los_Angeles", + "2023-03-12T10:05:06+00:00", + ), + ( + // Sunday, 5 November 2023, 02:00:00 clocks are turned backward 1 hour + // to Sunday, 5 November 2023, 01:00:00 local standard time instead, so + // 01:30:06 happens twice. The later instant, i.e. PST, is used, as + // PostgreSQL and DuckDB do. + "2023-11-05 01:30:06 America/Los_Angeles", + "2023-11-05T09:30:06+00:00", + ), ]; for (s, expected) in cases { @@ -60,18 +76,6 @@ fn test_parse_timezone_invalid() { "2023-01-01 04:05:06.789 +07:30:00", "Parser error: Invalid timezone \"+07:30:00\": failed to parse timezone", ), - ( - // Sunday, 12 March 2023, 02:00:00 clocks are turned forward 1 hour to - // Sunday, 12 March 2023, 03:00:00 local daylight time instead. - "2023-03-12 02:05:06 America/Los_Angeles", - "Parser error: Error parsing timestamp from '2023-03-12 02:05:06 America/Los_Angeles': error computing timezone offset", - ), - ( - // Sunday, 5 November 2023, 02:00:00 clocks are turned backward 1 hour to - // Sunday, 5 November 2023, 01:00:00 local standard time instead. - "2023-11-05 01:30:06 America/Los_Angeles", - "Parser error: Error parsing timestamp from '2023-11-05 01:30:06 America/Los_Angeles': error computing timezone offset", - ), ]; for (s, expected) in cases {
