Jefffrey commented on code in PR #11054: URL: https://github.com/apache/arrow-rs/pull/11054#discussion_r4387326402
########## arrow-cast/src/local_time.rs: ########## @@ -0,0 +1,107 @@ +// 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. +/// +/// The gap rule probes the offset 24 hours earlier. This is correct for the Review Comment: this section should be combined with the section above about `None` not being expected in practice as they describe the same thing ########## arrow-cast/src/parse.rs: ########## @@ -162,8 +163,28 @@ 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. +/// +/// These rules also apply to the CSV and JSON readers, which call this Review Comment: it feels odd to mention CSV and JSON here; it seems more appropriate to locate that in csv/json related code, as that is where users of csv/json would look for such documentation -- 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]
