Jefffrey commented on code in PR #11038:
URL: https://github.com/apache/arrow-rs/pull/11038#discussion_r4044095358
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -2626,15 +2633,70 @@ fn cast_numeric_to_binary<FROM: ArrowPrimitiveType, O:
OffsetSizeTrait>(
)?))
}
+/// 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.
+///
+/// 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()),
+ // The second offset of `Ambiguous` is the one that yields the later
instant.
+ LocalResult::Ambiguous(_, later) => Some(later.fix()),
Review Comment:
```suggestion
LocalResult::Ambiguous(_earlier, later) => Some(later.fix()),
```
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -2626,15 +2633,70 @@ fn cast_numeric_to_binary<FROM: ArrowPrimitiveType, O:
OffsetSizeTrait>(
)?))
}
+/// Returns the offset to use when interpreting `local` as a wall clock reading
+/// in `tz`, or `None` if it cannot be resolved.
Review Comment:
perhaps we should explicitly add a note here about how `None` is expected to
be a rare case, which shouldnt be possible with the current timezone database
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -2626,15 +2633,70 @@ fn cast_numeric_to_binary<FROM: ArrowPrimitiveType, O:
OffsetSizeTrait>(
)?))
}
+/// 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.
+///
+/// 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()),
+ // The second offset of `Ambiguous` is the one that yields the later
instant.
+ LocalResult::Ambiguous(_, 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,
+ // and it is worth stating both:
Review Comment:
```suggestion
// Two separate properties of the timezone database make this
sound:
```
--
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]
