[GitHub] [arrow-rs] tustvold commented on a diff in pull request #4201: Fix incorrect cast Timestamp with Timezone

[via GitHub]

tustvold commented on code in PR #4201:
URL: https://github.com/apache/arrow-rs/pull/4201#discussion_r1192296688


##########
arrow-cast/src/cast.rs:
##########
@@ -5978,6 +6050,174 @@ mod tests {
         assert!(b.is_err());
     }
 
+    // Cast Timestamp(_, None) -> Timestamp(_, Some(timezone))
+    #[test]
+    fn test_cast_timestamp_with_timezone_1() {
+        let string_array: Arc<dyn Array> = Arc::new(StringArray::from(vec![
+            Some("2000-01-01T00:00:00.123456789"),
+            Some("2010-01-01T00:00:00.123456789"),
+            None,
+        ]));
+        let to_type = DataType::Timestamp(TimeUnit::Nanosecond, None);
+        let timestamp_array = cast(&string_array, &to_type).unwrap();
+
+        let to_type = DataType::Timestamp(TimeUnit::Microsecond, 
Some("+0700".into()));
+        let timestamp_array = cast(&timestamp_array, &to_type).unwrap();
+
+        let string_array = cast(&timestamp_array, &DataType::Utf8).unwrap();
+        let result = 
string_array.as_any().downcast_ref::<StringArray>().unwrap();

Review Comment:
   ```suggestion
           let result = string_array.as_string::<i32>();
   ```
   
   And similar below



##########
arrow-cast/src/cast.rs:
##########
@@ -5978,6 +6050,174 @@ mod tests {
         assert!(b.is_err());
     }
 
+    // Cast Timestamp(_, None) -> Timestamp(_, Some(timezone))

Review Comment:
   These tests are brilliant :+1: 



##########
arrow-cast/src/cast.rs:
##########
@@ -3005,6 +3049,34 @@ fn cast_string_to_month_day_nano_interval<Offset: 
OffsetSizeTrait>(
     Ok(Arc::new(interval_array) as ArrayRef)
 }
 
+fn adjust_timestamp_to_timezone<
+    T: ArrowTimestampType,
+>(
+    array: PrimitiveArray<Int64Type>,
+    to_tz: &Tz,
+    cast_options: &CastOptions,
+) -> Result<PrimitiveArray<Int64Type>, ArrowError> {
+    let adjusted = if cast_options.safe {
+        array.unary_opt::<_, Int64Type>(|o| {
+            let local = as_datetime::<T>(o)?;
+            let offset = to_tz.offset_from_local_datetime(&local).single()?;
+            T::make_value(local - offset.fix())
+        })
+    } else {
+        let error_fn = || {
+            ArrowError::CastError(
+                "Cannot cast timezone to different timezone".to_string(),
+            )
+        };
+        array.try_unary::<_, Int64Type, _>(|o| {
+            let local = as_datetime::<T>(o).ok_or_else(error_fn)?;
+            let offset = 
to_tz.offset_from_local_datetime(&local).single().ok_or_else(error_fn)?;
+            T::make_value(local - offset.fix()).ok_or_else(error_fn)
+        })?
+    };

Review Comment:
   ```suggestion
       let adjust = |o| {
           let local = as_datetime::<T>(o)?;
               let offset = to_tz.offset_from_local_datetime(&local).single()?;
               T::make_value(local - offset.fix())
       };
       let adjusted = if cast_options.safe {
           array.unary_opt::<_, Int64Type>(adjust)
       } else {
           array.try_unary::<_, Int64Type, _>(|o| {
               adjust(o).ok_or_else(|| {
                   ArrowError::CastError(
                       "Cannot cast timezone to different timezone".to_string(),
                   )
               })
           })?
       };
   ```
   Perhaps?



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org