yinli-systems commented on code in PR #10907:
URL: https://github.com/apache/arrow-rs/pull/10907#discussion_r3918101256


##########
arrow-cast/src/cast/mod.rs:
##########
@@ -1818,32 +1818,48 @@ pub fn cast_with_options(
                 .unary::<_, Time64NanosecondType>(|x| x as i64 * (NANOSECONDS 
/ MILLISECONDS)),
         )),
 
-        (Time64(TimeUnit::Microsecond), Time32(TimeUnit::Second)) => 
Ok(Arc::new(
-            array
-                .as_primitive::<Time64MicrosecondType>()
-                .unary::<_, Time32SecondType>(|x| (x / MICROSECONDS) as i32),
-        )),
-        (Time64(TimeUnit::Microsecond), Time32(TimeUnit::Millisecond)) => 
Ok(Arc::new(
-            array
+        (Time64(TimeUnit::Microsecond), Time32(TimeUnit::Second)) => {
+            let array = array
                 .as_primitive::<Time64MicrosecondType>()
-                .unary::<_, Time32MillisecondType>(|x| (x / (MICROSECONDS / 
MILLISECONDS)) as i32),
-        )),
-        (Time64(TimeUnit::Microsecond), Time64(TimeUnit::Nanosecond)) => 
Ok(Arc::new(
-            array
+                .unary::<_, Int64Type>(|x| x / MICROSECONDS);
+            let array = cast_with_options(&array, &Int32, cast_options)?;

Review Comment:
   Addressed in 0fbc616bc. All four Time64-to-Time32 paths now scale and 
range-check directly in a single unary_opt/try_unary pass, so they no longer 
allocate an intermediate Int64Array. I also rebased onto current main. 
Validation: cargo test -p arrow-cast (380 unit tests and 12 doctests), cargo 
clippy -p arrow-cast --all-targets --all-features -- -D warnings, and cargo 
+stable fmt --all -- --check. Thanks for catching this.



-- 
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]

Reply via email to