kosiew commented on code in PR #11054:
URL: https://github.com/apache/arrow-rs/pull/11054#discussion_r4312799723
##########
arrow-cast/src/parse.rs:
##########
@@ -162,8 +163,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
Review Comment:
This change also affects the CSV and JSON readers, which call
`string_to_datetime` directly. Timestamps in DST gaps or overlaps that
previously failed will now be accepted. Could you mention this behavior in the
rustdoc and PR description? A reader-level regression test would also be useful.
##########
arrow-cast/src/parse.rs:
##########
@@ -3819,4 +3830,253 @@ 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() {
Review Comment:
Nit: Some named-zone cases are duplicated across the parser, cast, and
integration tests. Could you remove the two Los Angeles rows already covered by
`test_parse_timezone` and the separate
`test_cast_string_to_timezone_named_in_string_dst` test? The remaining tests
should still cover the distinct parser and cast behaviors.
##########
arrow-cast/src/local_time.rs:
##########
@@ -0,0 +1,100 @@
+// 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.
+///
+/// `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.
+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)?;
+ Some(tz.from_utc_datetime(&(*local - offset.fix())))
Review Comment:
The 24-hour probe assumes IANA timezone rules, but this helper also accepts
other `TimeZone` implementations. Could you clarify in the documentation that a
non-IANA timezone may return `None` or select an incorrect offset if its
transitions do not satisfy those assumptions?
##########
arrow-cast/src/parse.rs:
##########
@@ -3819,4 +3830,253 @@ 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",
+ ),
+ (
+ "2023-11-05 01:30:06 America/Los_Angeles",
+ 1_699_176_606,
+ "2023-11-05T09:30:06+00:00",
+ ),
+ (
+ "2023-03-12 02:05:06 America/Los_Angeles",
+ 1_678_615_506,
+ "2023-03-12T10:05:06+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_038_000_000_000_000 - 200_000_000_000
Review Comment:
Nit: Could you write the expected value as `1_710_037_800 * 1_000_000_000`
instead of subtracting two large numbers? That would make the timestamp easier
to verify, as in the existing cast test.
##########
arrow-cast/src/local_time.rs:
##########
@@ -0,0 +1,100 @@
+// 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.
+///
+/// `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.
+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)?;
+ Some(tz.from_utc_datetime(&(*local - offset.fix())))
Review Comment:
Nit: `from_utc_datetime` performs another timezone lookup even though we
already have the offset. Could you use
`local.checked_sub_offset(offset.fix()).map(|utc|
DateTime::from_naive_utc_and_offset(utc, offset))` instead? This avoids the
extra lookup and handles subtraction overflow without panicking.
--
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]
