You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2021/11/22 21:01:05 UTC
[arrow-rs] branch cherry_pick_9703f989 created (now c6518bb)
This is an automated email from the ASF dual-hosted git repository.
alamb pushed a change to branch cherry_pick_9703f989
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git.
at c6518bb Fix bug in temporal utilities due to DST being ignored. (#955)
This branch includes the following new commits:
new c6518bb Fix bug in temporal utilities due to DST being ignored. (#955)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[arrow-rs] 01/01: Fix bug in temporal utilities due to DST being ignored. (#955)
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch cherry_pick_9703f989
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit c6518bbf36a15371ccb8abdd2c686a28e556fc00
Author: Navin <na...@novemberkilo.com>
AuthorDate: Sat Nov 20 23:04:58 2021 +1100
Fix bug in temporal utilities due to DST being ignored. (#955)
* Check behaviour of temporal utilities for DST.
* Fix temporal util bug ignoring dst.
* Refactor macro for efficiency.
---
arrow/src/compute/kernels/temporal.rs | 92 ++++++++++++++++++-----------------
1 file changed, 48 insertions(+), 44 deletions(-)
diff --git a/arrow/src/compute/kernels/temporal.rs b/arrow/src/compute/kernels/temporal.rs
index 269f5cb..317f8bb 100644
--- a/arrow/src/compute/kernels/temporal.rs
+++ b/arrow/src/compute/kernels/temporal.rs
@@ -47,23 +47,44 @@ macro_rules! extract_component_from_array {
"Expected format [+-]XX:XX".to_string()
)
} else {
- let fixed_offset = match parse(&mut $parsed, $tz, StrftimeItems::new("%z")) {
+ let tz_parse_result = parse(&mut $parsed, $tz, StrftimeItems::new("%z"));
+ let fixed_offset_from_parsed = match tz_parse_result {
Ok(_) => match $parsed.to_fixed_offset() {
- Ok(fo) => fo,
+ Ok(fo) => Some(fo),
err => return_compute_error_with!("Invalid timezone", err),
},
- _ => match using_chrono_tz($tz) {
- Some(fo) => fo,
- err => return_compute_error_with!("Unable to parse timezone", err),
- },
+ _ => None,
};
+
for i in 0..$array.len() {
if $array.is_null(i) {
$builder.append_null()?;
} else {
- match $array.$using(i, fixed_offset) {
- Some(dt) => $builder.append_value(dt.$extract_fn() as i32)?,
- None => $builder.append_null()?,
+ match $array.value_as_datetime(i) {
+ Some(utc) => {
+ let fixed_offset = match fixed_offset_from_parsed {
+ Some(fo) => fo,
+ None => match using_chrono_tz_and_utc_naive_date_time(
+ $tz, utc,
+ ) {
+ Some(fo) => fo,
+ err => return_compute_error_with!(
+ "Unable to parse timezone",
+ err
+ ),
+ },
+ };
+ match $array.$using(i, fixed_offset) {
+ Some(dt) => {
+ $builder.append_value(dt.$extract_fn() as i32)?
+ }
+ None => $builder.append_null()?,
+ }
+ }
+ err => return_compute_error_with!(
+ "Unable to read value as datetime",
+ err
+ ),
}
}
}
@@ -77,31 +98,14 @@ macro_rules! return_compute_error_with {
};
}
-/// Parse the given string into a string representing fixed-offset
-#[cfg(not(feature = "chrono-tz"))]
-pub fn using_chrono_tz(_: &str) -> Option<FixedOffset> {
- None
-}
-
-/// Parse the given string into a string representing fixed-offset
-#[cfg(feature = "chrono-tz")]
-pub fn using_chrono_tz(tz: &str) -> Option<FixedOffset> {
- use chrono::{Offset, TimeZone};
- tz.parse::<chrono_tz::Tz>()
- .map(|tz| {
- tz.offset_from_utc_datetime(&chrono::NaiveDateTime::from_timestamp(0, 0))
- .fix()
- })
- .ok()
-}
-
#[cfg(not(feature = "chrono-tz"))]
pub fn using_chrono_tz_and_utc_naive_date_time(
_tz: &str,
_utc: chrono::NaiveDateTime,
) -> Option<FixedOffset> {
- Some(FixedOffset::east(0))
+ None
}
+
/// Parse the given string into a string representing fixed-offset that is correct as of the given
/// UTC NaiveDateTime.
/// Note that the offset is function of time and can vary depending on whether daylight savings is
@@ -448,6 +452,22 @@ mod tests {
assert_eq!(15, b.value(0));
}
+ #[cfg(feature = "chrono-tz")]
+ #[test]
+ fn test_temporal_array_timestamp_hour_with_dst_timezone_using_chrono_tz() {
+ //
+ // 1635577147 converts to 2021-10-30 17:59:07 in time zone Australia/Sydney (AEDT)
+ // The offset (difference to UTC) is +11:00. Note that daylight savings is in effect on 2021-10-30.
+ // When daylight savings is not in effect, Australia/Sydney has an offset difference of +10:00.
+
+ let a = TimestampMillisecondArray::from_opt_vec(
+ vec![Some(1635577147000)],
+ Some("Australia/Sydney".to_string()),
+ );
+ let b = hour(&a).unwrap();
+ assert_eq!(17, b.value(0));
+ }
+
#[cfg(not(feature = "chrono-tz"))]
#[test]
fn test_temporal_array_timestamp_hour_with_timezone_using_chrono_tz() {
@@ -462,22 +482,6 @@ mod tests {
#[cfg(feature = "chrono-tz")]
#[test]
- fn test_using_chrono_tz() {
- let sydney_offset = FixedOffset::east(10 * 60 * 60);
- assert_eq!(
- using_chrono_tz(&"Australia/Sydney".to_string()),
- Some(sydney_offset)
- );
-
- let nyc_offset = FixedOffset::west(5 * 60 * 60);
- assert_eq!(
- using_chrono_tz(&"America/New_York".to_string()),
- Some(nyc_offset)
- );
- }
-
- #[cfg(feature = "chrono-tz")]
- #[test]
fn test_using_chrono_tz_and_utc_naive_date_time() {
let sydney_tz = "Australia/Sydney".to_string();
let sydney_offset_without_dst = FixedOffset::east(10 * 60 * 60);