viirya commented on code in PR #11482:
URL: https://github.com/apache/arrow-rs/pull/11482#discussion_r4238824894
##########
arrow-array/src/timezone.rs:
##########
@@ -48,7 +48,360 @@ fn parse_fixed_offset(tz: &str) -> Option<FixedOffset> {
}
}
-#[cfg(feature = "chrono-tz")]
+#[cfg(feature = "jiff")]
+mod private {
+ use super::*;
+ use chrono::offset::TimeZone;
+ use chrono::{Datelike, LocalResult, NaiveDate, NaiveDateTime, NaiveTime,
Offset, Timelike};
+ use jiff::Timestamp;
+ use jiff::civil::DateTime as JiffDateTime;
+ use jiff::tz::{AmbiguousOffset, TimeZone as JiffTimeZone};
+ use std::collections::HashMap;
+ use std::fmt::Display;
+ use std::str::FromStr;
+ use std::sync::{OnceLock, RwLock};
+
+ /// An [`Offset`] for [`Tz`]
+ #[derive(Debug, Copy, Clone)]
+ pub struct TzOffset {
+ tz: Tz,
+ offset: FixedOffset,
+ }
+
+ impl std::fmt::Display for TzOffset {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.offset.fmt(f)
+ }
+ }
+
+ impl Offset for TzOffset {
+ fn fix(&self) -> FixedOffset {
+ self.offset
+ }
+ }
+
+ /// A named timezone resolved by jiff, together with the name it was
parsed from.
+ ///
+ /// [`Tz`] is `Copy` whereas [`JiffTimeZone`] is reference counted, so
each distinct name is
+ /// resolved once and leaked to give `Tz` a `'static` reference. The set
of names jiff accepts
+ /// is bounded by the bundled database, so the leaked memory is bounded
too, and the hot path
+ /// (offset lookup) takes no lock.
+ #[derive(Debug)]
+ struct NamedTz {
+ name: Box<str>,
+ tz: JiffTimeZone,
+ }
+
+ fn named_tz(name: &str) -> Result<&'static NamedTz, ArrowError> {
+ static CACHE: OnceLock<RwLock<HashMap<Box<str>, &'static NamedTz>>> =
OnceLock::new();
+ let cache = CACHE.get_or_init(Default::default);
+ if let Some(tz) = cache.read().unwrap().get(name) {
+ return Ok(tz);
+ }
+ // Same message as the chrono-tz path, so callers see one error
regardless of feature
+ let tz = JiffTimeZone::get(name).map_err(|_| {
Review Comment:
This goes through jiff's global `db()`. If anything else in the dependency
graph enables jiff's default features (for example, `opendal-core` already
depends on jiff in this workspace's lockfile), jiff reads `/usr/share/zoneinfo`
/ `$TZDIR` before the bundle. With `TZDIR` pointing at a directory whose
`America/Los_Angeles` is a copy of `Asia/Tokyo`, `Tz` returned +09:00, and a
name only present there (`Fake_Zone`) parsed successfully. Could this use
`jiff::tz::TimeZoneDatabase::bundled()` explicitly, held in a static?
##########
arrow-array/src/timezone.rs:
##########
@@ -48,7 +48,360 @@ fn parse_fixed_offset(tz: &str) -> Option<FixedOffset> {
}
}
-#[cfg(feature = "chrono-tz")]
+#[cfg(feature = "jiff")]
+mod private {
+ use super::*;
+ use chrono::offset::TimeZone;
+ use chrono::{Datelike, LocalResult, NaiveDate, NaiveDateTime, NaiveTime,
Offset, Timelike};
+ use jiff::Timestamp;
+ use jiff::civil::DateTime as JiffDateTime;
+ use jiff::tz::{AmbiguousOffset, TimeZone as JiffTimeZone};
+ use std::collections::HashMap;
+ use std::fmt::Display;
+ use std::str::FromStr;
+ use std::sync::{OnceLock, RwLock};
+
+ /// An [`Offset`] for [`Tz`]
+ #[derive(Debug, Copy, Clone)]
+ pub struct TzOffset {
+ tz: Tz,
+ offset: FixedOffset,
+ }
+
+ impl std::fmt::Display for TzOffset {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.offset.fmt(f)
+ }
+ }
+
+ impl Offset for TzOffset {
+ fn fix(&self) -> FixedOffset {
+ self.offset
+ }
+ }
+
+ /// A named timezone resolved by jiff, together with the name it was
parsed from.
+ ///
+ /// [`Tz`] is `Copy` whereas [`JiffTimeZone`] is reference counted, so
each distinct name is
+ /// resolved once and leaked to give `Tz` a `'static` reference. The set
of names jiff accepts
+ /// is bounded by the bundled database, so the leaked memory is bounded
too, and the hot path
+ /// (offset lookup) takes no lock.
+ #[derive(Debug)]
+ struct NamedTz {
+ name: Box<str>,
+ tz: JiffTimeZone,
+ }
+
+ fn named_tz(name: &str) -> Result<&'static NamedTz, ArrowError> {
+ static CACHE: OnceLock<RwLock<HashMap<Box<str>, &'static NamedTz>>> =
OnceLock::new();
+ let cache = CACHE.get_or_init(Default::default);
+ if let Some(tz) = cache.read().unwrap().get(name) {
+ return Ok(tz);
+ }
+ // Same message as the chrono-tz path, so callers see one error
regardless of feature
+ let tz = JiffTimeZone::get(name).map_err(|_| {
+ ArrowError::ParseError(format!(
+ "Invalid timezone \"{name}\": failed to parse timezone"
+ ))
+ })?;
+ let mut cache = cache.write().unwrap();
+ Ok(cache.entry(name.into()).or_insert_with(|| {
Review Comment:
jiff's lookup is ASCII case-insensitive, but the cache is keyed by the input
string, so every casing is resolved and leaked separately. All 4096 case
variants of the first 12 chars of `america/los_angeles` were accepted and
leaked. `string_to_datetime` (arrow-cast/src/parse.rs:221) parses the timezone
suffix of each row, so CSV/JSON/cast input can reach this. It's also a
behaviour change: chrono-tz's `FromStr` is case-sensitive, while this accepts
`utc`, `america/new_york`, `Etc/Unknown` and `Factory`, and `Display` echoes
the non-canonical casing. Requiring `tz.iana_name() == Some(name)` (or
keying/displaying by the canonical name) would fix both.
##########
arrow-array/src/timezone.rs:
##########
@@ -48,7 +48,360 @@ fn parse_fixed_offset(tz: &str) -> Option<FixedOffset> {
}
}
-#[cfg(feature = "chrono-tz")]
+#[cfg(feature = "jiff")]
+mod private {
+ use super::*;
+ use chrono::offset::TimeZone;
+ use chrono::{Datelike, LocalResult, NaiveDate, NaiveDateTime, NaiveTime,
Offset, Timelike};
+ use jiff::Timestamp;
+ use jiff::civil::DateTime as JiffDateTime;
+ use jiff::tz::{AmbiguousOffset, TimeZone as JiffTimeZone};
+ use std::collections::HashMap;
+ use std::fmt::Display;
+ use std::str::FromStr;
+ use std::sync::{OnceLock, RwLock};
+
+ /// An [`Offset`] for [`Tz`]
+ #[derive(Debug, Copy, Clone)]
+ pub struct TzOffset {
+ tz: Tz,
+ offset: FixedOffset,
+ }
+
+ impl std::fmt::Display for TzOffset {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.offset.fmt(f)
+ }
+ }
+
+ impl Offset for TzOffset {
+ fn fix(&self) -> FixedOffset {
+ self.offset
+ }
+ }
+
+ /// A named timezone resolved by jiff, together with the name it was
parsed from.
+ ///
+ /// [`Tz`] is `Copy` whereas [`JiffTimeZone`] is reference counted, so
each distinct name is
+ /// resolved once and leaked to give `Tz` a `'static` reference. The set
of names jiff accepts
+ /// is bounded by the bundled database, so the leaked memory is bounded
too, and the hot path
+ /// (offset lookup) takes no lock.
+ #[derive(Debug)]
+ struct NamedTz {
+ name: Box<str>,
+ tz: JiffTimeZone,
+ }
+
+ fn named_tz(name: &str) -> Result<&'static NamedTz, ArrowError> {
+ static CACHE: OnceLock<RwLock<HashMap<Box<str>, &'static NamedTz>>> =
OnceLock::new();
+ let cache = CACHE.get_or_init(Default::default);
+ if let Some(tz) = cache.read().unwrap().get(name) {
+ return Ok(tz);
+ }
+ // Same message as the chrono-tz path, so callers see one error
regardless of feature
+ let tz = JiffTimeZone::get(name).map_err(|_| {
+ ArrowError::ParseError(format!(
+ "Invalid timezone \"{name}\": failed to parse timezone"
+ ))
+ })?;
+ let mut cache = cache.write().unwrap();
+ Ok(cache.entry(name.into()).or_insert_with(|| {
+ Box::leak(Box::new(NamedTz {
+ name: name.into(),
+ tz,
+ }))
+ }))
+ }
+
+ /// An Arrow [`TimeZone`]
+ #[derive(Debug, Copy, Clone)]
+ pub struct Tz(TzInner);
+
+ #[derive(Debug, Copy, Clone)]
+ enum TzInner {
+ Timezone(&'static NamedTz),
+ Offset(FixedOffset),
+ }
+
+ impl FromStr for Tz {
+ type Err = ArrowError;
+
+ fn from_str(tz: &str) -> Result<Self, Self::Err> {
+ match parse_fixed_offset(tz) {
+ Some(offset) => Ok(Self(TzInner::Offset(offset))),
+ None => Ok(Self(TzInner::Timezone(named_tz(tz)?))),
+ }
+ }
+ }
+
+ impl Display for Tz {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self.0 {
+ TzInner::Timezone(tz) => f.write_str(&tz.name),
+ TzInner::Offset(offset) => offset.fmt(f),
+ }
+ }
+ }
+
+ fn fixed_offset(offset: jiff::tz::Offset) -> FixedOffset {
+ // Real timezone data never holds an offset of a day or more, so this
cannot fail
+ FixedOffset::east_opt(offset.seconds()).expect("timezone offset within
a day")
Review Comment:
jiff allows offsets up to ±25:59:59, so this holds for the bundled data but
not for an arbitrary TZif file, which can be loaded through the host zoneinfo
path above. Using the bundled db explicitly would make the comment true.
Otherwise, clamping or returning UTC would be safer than panicking.
##########
.github/workflows/arrow.yml:
##########
@@ -77,6 +77,8 @@ jobs:
run: cargo test -p arrow
- name: Test arrow except pyarrow
run: cargo test -p arrow
--features=force_validate,prettyprint,ipc_compression,ffi,chrono-tz
+ - name: Test arrow with named timezones resolved by jiff
Review Comment:
With this PR, `cargo test -p arrow-array --all-features` (line 49) selects
jiff, and the other arrow-array steps enable neither feature. The chrono-tz
`private` module and its tests are no longer compiled, run, or linted (clippy
also uses `--all-features`). Could you add a `cargo test -p arrow-array
--features chrono-tz` step and a matching clippy step?
##########
arrow-array/Cargo.toml:
##########
@@ -55,6 +55,9 @@ arrow-schema = { workspace = true }
arrow-data = { workspace = true }
chrono = { workspace = true }
chrono-tz = { version = "0.10", optional = true }
+# `tzdb-bundle-always` compiles the IANA database into the crate, as chrono-tz
does, so results
Review Comment:
This only holds if no other crate in the build enables jiff's
`tzdb-zoneinfo` (see the comment on `named_tz`). The PR description also lists
only `std + tzdb-bundle-always`, while `tz-fat` and `perf-inline` are enabled
too.
##########
arrow/benches/named_timezone.rs:
##########
@@ -0,0 +1,125 @@
+// 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.
+
+//! Benchmarks for named (IANA) timezones, which the `chrono-tz` and `jiff`
features resolve.
+//!
+//! Run with `--features chrono-tz` and again with `--features chrono-tz,jiff`
to compare the two
+//! implementations behind [`Tz`].
+
+use arrow::array::timezone::Tz;
+use arrow::array::{TimestampMicrosecondArray, TimestampMillisecondArray};
+use arrow::compute::{DatePart, cast, date_part};
+use arrow::datatypes::DataType;
+use chrono::{DateTime, NaiveDateTime, TimeZone};
+use criterion::{Criterion, criterion_group, criterion_main};
+use std::hint::black_box;
+
+const ZONES: [&str; 4] = ["UTC", "America/Los_Angeles", "Europe/London",
"Asia/Tokyo"];
+
+/// Instant ranges in microseconds. Fat TZif data, and so jiff's transition
tables, stop at 2038;
Review Comment:
2038–2100 is still inside chrono-tz's precomputed table: LA offsets match
jiff exactly until 2100. So the ~2x there is the cost of jiff evaluating the
POSIX rule versus a table lookup, not chrono-tz skipping the rule. That range
is a pure slowdown with no correctness gain, which seems worth saying in the
description. Also, `tz_parse` is single-threaded, so it doesn't capture
contention on the global `RwLock` when a timezone is parsed per row.
##########
arrow-array/src/timezone.rs:
##########
@@ -48,7 +48,360 @@ fn parse_fixed_offset(tz: &str) -> Option<FixedOffset> {
}
}
-#[cfg(feature = "chrono-tz")]
+#[cfg(feature = "jiff")]
+mod private {
+ use super::*;
+ use chrono::offset::TimeZone;
+ use chrono::{Datelike, LocalResult, NaiveDate, NaiveDateTime, NaiveTime,
Offset, Timelike};
+ use jiff::Timestamp;
+ use jiff::civil::DateTime as JiffDateTime;
+ use jiff::tz::{AmbiguousOffset, TimeZone as JiffTimeZone};
+ use std::collections::HashMap;
+ use std::fmt::Display;
+ use std::str::FromStr;
+ use std::sync::{OnceLock, RwLock};
+
+ /// An [`Offset`] for [`Tz`]
+ #[derive(Debug, Copy, Clone)]
+ pub struct TzOffset {
+ tz: Tz,
+ offset: FixedOffset,
+ }
+
+ impl std::fmt::Display for TzOffset {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.offset.fmt(f)
+ }
+ }
+
+ impl Offset for TzOffset {
+ fn fix(&self) -> FixedOffset {
+ self.offset
+ }
+ }
+
+ /// A named timezone resolved by jiff, together with the name it was
parsed from.
+ ///
+ /// [`Tz`] is `Copy` whereas [`JiffTimeZone`] is reference counted, so
each distinct name is
+ /// resolved once and leaked to give `Tz` a `'static` reference. The set
of names jiff accepts
+ /// is bounded by the bundled database, so the leaked memory is bounded
too, and the hot path
+ /// (offset lookup) takes no lock.
+ #[derive(Debug)]
+ struct NamedTz {
+ name: Box<str>,
+ tz: JiffTimeZone,
+ }
+
+ fn named_tz(name: &str) -> Result<&'static NamedTz, ArrowError> {
+ static CACHE: OnceLock<RwLock<HashMap<Box<str>, &'static NamedTz>>> =
OnceLock::new();
+ let cache = CACHE.get_or_init(Default::default);
+ if let Some(tz) = cache.read().unwrap().get(name) {
+ return Ok(tz);
+ }
+ // Same message as the chrono-tz path, so callers see one error
regardless of feature
+ let tz = JiffTimeZone::get(name).map_err(|_| {
+ ArrowError::ParseError(format!(
+ "Invalid timezone \"{name}\": failed to parse timezone"
+ ))
+ })?;
+ let mut cache = cache.write().unwrap();
+ Ok(cache.entry(name.into()).or_insert_with(|| {
+ Box::leak(Box::new(NamedTz {
+ name: name.into(),
+ tz,
+ }))
+ }))
+ }
+
+ /// An Arrow [`TimeZone`]
+ #[derive(Debug, Copy, Clone)]
+ pub struct Tz(TzInner);
+
+ #[derive(Debug, Copy, Clone)]
+ enum TzInner {
+ Timezone(&'static NamedTz),
+ Offset(FixedOffset),
+ }
+
+ impl FromStr for Tz {
+ type Err = ArrowError;
+
+ fn from_str(tz: &str) -> Result<Self, Self::Err> {
+ match parse_fixed_offset(tz) {
+ Some(offset) => Ok(Self(TzInner::Offset(offset))),
+ None => Ok(Self(TzInner::Timezone(named_tz(tz)?))),
+ }
+ }
+ }
+
+ impl Display for Tz {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self.0 {
+ TzInner::Timezone(tz) => f.write_str(&tz.name),
+ TzInner::Offset(offset) => offset.fmt(f),
+ }
+ }
+ }
+
+ fn fixed_offset(offset: jiff::tz::Offset) -> FixedOffset {
+ // Real timezone data never holds an offset of a day or more, so this
cannot fail
+ FixedOffset::east_opt(offset.seconds()).expect("timezone offset within
a day")
+ }
+
+ /// The instant jiff should consult for a UTC datetime. jiff covers years
-9999 to 9999;
+ /// anything outside that range takes the offset in force at the nearest
covered instant.
+ fn jiff_timestamp(utc: &NaiveDateTime) -> Timestamp {
+ let seconds = utc.and_utc().timestamp();
+ Timestamp::from_second(seconds).unwrap_or(if seconds < 0 {
+ Timestamp::MIN
+ } else {
+ Timestamp::MAX
+ })
+ }
+
+ /// The civil datetime jiff should resolve for a local datetime, or `None`
when the year is
+ /// outside jiff's range. Sub-second precision is irrelevant to the offset
in force.
+ fn jiff_civil(local: &NaiveDateTime) -> Option<JiffDateTime> {
+ JiffDateTime::new(
+ i16::try_from(local.year()).ok()?,
+ local.month() as i8,
+ local.day() as i8,
+ local.hour() as i8,
+ local.minute() as i8,
+ local.second() as i8,
+ 0,
+ )
+ .ok()
+ }
+
+ impl Tz {
+ fn offset(&self, offset: FixedOffset) -> TzOffset {
+ TzOffset { tz: *self, offset }
+ }
+ }
+
+ impl TimeZone for Tz {
+ type Offset = TzOffset;
+
+ fn from_offset(offset: &Self::Offset) -> Self {
+ offset.tz
+ }
+
+ fn offset_from_local_date(&self, local: &NaiveDate) ->
LocalResult<Self::Offset> {
+ match self.0 {
+ TzInner::Offset(tz) => tz
+ .offset_from_local_date(local)
+ .map(|x| self.offset(x.fix())),
+ TzInner::Timezone(_) => {
+ // A date has no single offset on a day with a transition.
As chrono-tz
+ // does, prefer any offset that was in force at some point
during the day
+ // over reporting the ambiguity, so that unambiguous times
on such a day
+ // still resolve.
+ let earliest =
self.offset_from_local_datetime(&local.and_time(NaiveTime::MIN));
+ let latest =
+ self.offset_from_local_datetime(&local.and_hms_opt(23,
59, 59).unwrap());
+ match (earliest, latest) {
+ (result @ LocalResult::Single(_), _) => result,
+ (_, result @ LocalResult::Single(_)) => result,
+ (LocalResult::Ambiguous(offset, _), _) =>
LocalResult::Single(offset),
+ (_, LocalResult::Ambiguous(offset, _)) =>
LocalResult::Single(offset),
+ (LocalResult::None, LocalResult::None) =>
LocalResult::None,
+ }
+ }
+ }
+ }
+
+ fn offset_from_local_datetime(&self, local: &NaiveDateTime) ->
LocalResult<Self::Offset> {
+ match self.0 {
+ TzInner::Offset(tz) => tz
+ .offset_from_local_datetime(local)
+ .map(|x| self.offset(x.fix())),
+ TzInner::Timezone(tz) => match jiff_civil(local) {
+ None =>
LocalResult::Single(self.offset_from_utc_datetime(local)),
+ Some(dt) => match
tz.tz.to_ambiguous_timestamp(dt).offset() {
+ AmbiguousOffset::Unambiguous { offset } => {
+
LocalResult::Single(self.offset(fixed_offset(offset)))
+ }
+ AmbiguousOffset::Gap { .. } => LocalResult::None,
+ AmbiguousOffset::Fold { before, after } =>
LocalResult::Ambiguous(
+ self.offset(fixed_offset(before)),
+ self.offset(fixed_offset(after)),
+ ),
+ },
+ },
+ }
+ }
+
+ fn offset_from_utc_date(&self, utc: &NaiveDate) -> Self::Offset {
+ self.offset_from_utc_datetime(&utc.and_time(NaiveTime::MIN))
+ }
+
+ fn offset_from_utc_datetime(&self, utc: &NaiveDateTime) ->
Self::Offset {
+ match self.0 {
+ TzInner::Offset(tz) =>
self.offset(tz.offset_from_utc_datetime(utc).fix()),
+ TzInner::Timezone(tz) => {
+
self.offset(fixed_offset(tz.tz.to_offset(jiff_timestamp(utc))))
+ }
+ }
+ }
+ }
+
+ #[cfg(test)]
+ mod tests {
+ use super::*;
+ use chrono::{Timelike, Utc};
+
+ #[test]
+ fn test_with_timezone() {
+ let vals = [
+ Utc.timestamp_millis_opt(37800000).unwrap(),
+ Utc.timestamp_millis_opt(86339000).unwrap(),
+ ];
+
+ assert_eq!(10, vals[0].hour());
+ assert_eq!(23, vals[1].hour());
+
+ let tz: Tz = "America/Los_Angeles".parse().unwrap();
+
+ assert_eq!(2, vals[0].with_timezone(&tz).hour());
+ assert_eq!(15, vals[1].with_timezone(&tz).hour());
+ }
+
+ #[test]
+ fn test_dst_transitions_from_utc() {
+ let tz: Tz = "Australia/Sydney".parse().unwrap();
+ let standard = FixedOffset::east_opt(10 * 60 * 60).unwrap();
+ let daylight = FixedOffset::east_opt(11 * 60 * 60).unwrap();
+ let at = |y, m, d, h, min| {
+ NaiveDate::from_ymd_opt(y, m, d)
+ .unwrap()
+ .and_hms_opt(h, min, 0)
+ .unwrap()
+ };
+ // Daylight time ended 2021-04-04T03:00+11:00, which is
2021-04-03T16:00Z
+ assert_eq!(
+ tz.offset_from_utc_datetime(&at(2021, 4, 3, 15, 30)).fix(),
+ daylight
+ );
+ assert_eq!(
+ tz.offset_from_utc_datetime(&at(2021, 4, 3, 16, 30)).fix(),
+ standard
+ );
+ // Daylight time started 2021-10-03T02:00+10:00, which is
2021-10-02T16:00Z
+ assert_eq!(
+ tz.offset_from_utc_datetime(&at(2021, 10, 2, 15, 30)).fix(),
+ standard
+ );
+ assert_eq!(
+ tz.offset_from_utc_datetime(&at(2021, 10, 2, 16, 30)).fix(),
+ daylight
+ );
+ }
+
+ #[test]
+ fn test_dst_transitions_from_local() {
+ let tz: Tz = "America/New_York".parse().unwrap();
+ let eastern_standard = FixedOffset::west_opt(5 * 60 * 60).unwrap();
+ let eastern_daylight = FixedOffset::west_opt(4 * 60 * 60).unwrap();
+ let at = |m, d, h, min| {
+ NaiveDate::from_ymd_opt(2024, m, d)
+ .unwrap()
+ .and_hms_opt(h, min, 0)
+ .unwrap()
+ };
+ match tz.offset_from_local_datetime(&at(1, 15, 12, 0)) {
+ LocalResult::Single(offset) => assert_eq!(offset.fix(),
eastern_standard),
+ other => panic!("expected a single offset, got {other:?}"),
+ }
+ // 02:30 on 2024-03-10 was skipped when clocks went forward
+ assert!(matches!(
+ tz.offset_from_local_datetime(&at(3, 10, 2, 30)),
+ LocalResult::None
+ ));
+ // 01:30 on 2024-11-03 happened twice; the daylight reading comes
first
+ match tz.offset_from_local_datetime(&at(11, 3, 1, 30)) {
+ LocalResult::Ambiguous(first, second) => {
+ assert_eq!(first.fix(), eastern_daylight);
+ assert_eq!(second.fix(), eastern_standard);
+ }
+ other => panic!("expected an ambiguous offset, got {other:?}"),
+ }
+ // A date on a transition day still resolves to an offset in force
that day
+ assert!(matches!(
+ tz.offset_from_local_date(&NaiveDate::from_ymd_opt(2024, 3,
10).unwrap()),
+ LocalResult::Single(_)
+ ));
+ }
+
+ /// chrono-tz stops applying daylight saving after its last
precomputed transition,
+ /// around 2099. jiff applies the zone's rules indefinitely, as
`java.time` and libc do.
+ #[test]
+ fn test_dst_applies_after_2099() {
+ let tz: Tz = "America/Los_Angeles".parse().unwrap();
+ let pacific_standard = FixedOffset::west_opt(8 * 60 * 60).unwrap();
+ let pacific_daylight = FixedOffset::west_opt(7 * 60 * 60).unwrap();
+ for year in [2024, 2100, 2300, 9999] {
+ let summer = NaiveDate::from_ymd_opt(year, 7, 1)
+ .unwrap()
+ .and_hms_opt(12, 0, 0)
+ .unwrap();
+ let winter = NaiveDate::from_ymd_opt(year, 1, 1)
+ .unwrap()
+ .and_hms_opt(12, 0, 0)
+ .unwrap();
+ assert_eq!(
+ tz.offset_from_utc_datetime(&summer).fix(),
+ pacific_daylight,
+ "summer {year}"
+ );
+ assert_eq!(
+ tz.offset_from_utc_datetime(&winter).fix(),
+ pacific_standard,
+ "winter {year}"
+ );
+ match tz.offset_from_local_datetime(&summer) {
+ LocalResult::Single(offset) => {
+ assert_eq!(offset.fix(), pacific_daylight, "local
summer {year}")
+ }
+ other => panic!("expected a single offset for {year}, got
{other:?}"),
+ }
+ }
+ }
+
+ #[test]
+ fn test_beyond_jiff_range() {
+ let tz: Tz = "America/Los_Angeles".parse().unwrap();
+ let far = NaiveDate::from_ymd_opt(20_000, 7, 1)
+ .unwrap()
+ .and_hms_opt(12, 0, 0)
+ .unwrap();
+ // Falls back to the offset in force at jiff's last covered
instant rather than failing
+ let utc = tz.offset_from_utc_datetime(&far).fix();
+ assert!(utc.local_minus_utc() == -8 * 60 * 60 ||
utc.local_minus_utc() == -7 * 60 * 60);
Review Comment:
The clamp deterministically gives −08:00 here, since jiff's max timestamp is
in December, so the assertion could be exact. On open question 3:
`offset_from_utc_datetime` is infallible, so an "error" would have to be a
panic. Clamping seems like the only workable option within the trait.
--
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]
