andygrove commented on code in PR #11482:
URL: https://github.com/apache/arrow-rs/pull/11482#discussion_r4249004378
##########
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:
Good catch. I reproduced it with jiff's default features enabled alongside
and `TZDIR` pointing at a fake directory. `America/Los_Angeles` came back as
+09:00, `Fake_Zone` parsed, and `US/Pacific` failed: jiff uses the zoneinfo
database in place of the bundle, not just ahead of it. Fixed in dafbd244c.
Names are now resolved through `TimeZoneDatabase::bundled()`, held in a static.
With the same setup, LA is back to -08:00, `Fake_Zone` is rejected, and
`US/Pacific` resolves from the bundle.
##########
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:
Fixed in dafbd244c. A name now has to match a bundled name exactly, so
`utc`, `america/new_york` and `Etc/Unknown` are rejected, and `Display` always
shows the canonical name. I also reject `Factory`: it's the only name jiff-tzdb
0.1.8 has that chrono-tz 0.10.4 doesn't, so both features now accept exactly
the same names.
I replaced the cache instead of re-keying it. It's now a map built once with
a slot per bundled name, each a `OnceLock` resolved on first use, and `Tz`
points into it. Memory is bounded by the database whatever the input, nothing
is leaked, and since the keys never change, parsing takes no lock. With ahash
for the lookup (13b912715), parsing a name takes 6–8 ns, against 11–13 ns for
chrono-tz.
##########
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:
Now that lookups only use the bundled database, the comment holds, and it
says so. To check, I walked every transition of all 598 bundled zones through
2200, plus the offsets at jiff's min and max instants. The largest absolute
offset is 15:56:08 (Asia/Manila's local mean time before 1845), so the `expect`
can't fire.
##########
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:
Made it exact in dafbd244c and covered the other end too: year -20000 clamps
to `Timestamp::MIN` and gets LA's local mean time, -07:52:58, on both the UTC
and the local path. Agreed on question 3. The trait methods are infallible, so
clamping it is, and I've dropped the question from the description.
##########
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:
With the explicit bundled database the comment holds whatever else is in the
build. I reworded it to say so and to note that `tz-fat` and `perf-inline` are
jiff defaults that speed up offset lookups (dafbd244c). I fixed the description
too.
##########
.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:
Added in ee30fb3e7: `cargo test -p arrow-array --features=chrono-tz` in the
arrow-array test step and `cargo clippy -p arrow-array --all-targets --features
chrono-tz -- -D warnings` in the clippy step.
##########
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:
You're right. Sampling hourly from 2038 to 2100, chrono-tz and jiff agree
exactly for all four zones in the bench, so that range is a pure slowdown: jiff
evaluating the POSIX rule against chrono-tz reading its table. I fixed the
comment and the description, and 1ad49e5e3 adds a 2100–2300 range, which is
where chrono-tz freezes the offset and evaluating the rule changes results. The
parse path no longer takes a lock (see the reply on `named_tz`) and is now
faster than chrono-tz's, so there's no contention left for a multithreaded
parse bench to show.
--
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]
