Fix temporal util bug ignoring dst.

apache · Nov 18, 2021 · 7e0e187 · 7e0e187
1 parent b340d2c
commit 7e0e187
Showing 1 changed file with 37 additions and 48 deletions.
diff --git a/arrow/src/compute/kernels/temporal.rs b/arrow/src/compute/kernels/temporal.rs
@@ -47,23 +47,45 @@ macro_rules! extract_component_from_array {
                 "Expected format [+-]XX:XX".to_string()
             )
         } else {
-            let fixed_offset = match parse(&mut $parsed, $tz, StrftimeItems::new("%z")) {
-                Ok(_) => match $parsed.to_fixed_offset() {
-                    Ok(fo) => 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),
-                },
-            };
             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 parse(
+                                &mut $parsed,
+                                $tz,
+                                StrftimeItems::new("%z"),
+                            ) {
+                                Ok(_) => match $parsed.to_fixed_offset() {
+                                    Ok(fo) => fo,
+                                    err => return_compute_error_with!(
+                                        "Invalid timezone",
+                                        err
+                                    ),
+                                },
+                                _ => 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 +99,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
@@ -476,22 +481,6 @@ mod tests {
         assert!(matches!(hour(&a), Err(ArrowError::ComputeError(_))))
     }
 
-    #[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() {