Jefffrey commented on code in PR #5337:
URL: https://github.com/apache/arrow-rs/pull/5337#discussion_r1468362207


##########
arrow-arith/src/temporal.rs:
##########
@@ -210,8 +320,48 @@ impl ExtractDatePartExt for 
PrimitiveArray<Time64MicrosecondType> {
 impl ExtractDatePartExt for PrimitiveArray<Time64NanosecondType> {
     fn date_part(&self, part: DatePart) -> Result<Int32Array, ArrowError> {
         match part {
-            DatePart::Hour => Ok(self.unary_opt(|d| 
time64ns_to_time(d).map(|c| c.hour() as i32))),
-            // TODO expand support for Time types, see: 
https://github.com/apache/arrow-rs/issues/5261
+            DatePart::Hour => Ok(self.unary_opt(|ns| {
+                if (0..NANOSECONDS_IN_DAY).contains(&ns) {
+                    Some((ns / 3_600 / NANOSECONDS) as i32)
+                } else {
+                    None
+                }
+            })),
+            DatePart::Minute => Ok(self.unary_opt(|ns| {
+                if (0..NANOSECONDS_IN_DAY).contains(&ns) {
+                    Some(((ns / 60 / NANOSECONDS) % 60) as i32)
+                } else {
+                    None
+                }
+            })),
+            DatePart::Second => Ok(self.unary_opt(|ns| {
+                if (0..NANOSECONDS_IN_DAY).contains(&ns) {
+                    Some(((ns / NANOSECONDS) % 60) as i32)
+                } else {
+                    None
+                }
+            })),
+            DatePart::Millisecond => Ok(self.unary_opt(|ns| {
+                if (0..NANOSECONDS_IN_DAY).contains(&ns) {
+                    Some(((ns % NANOSECONDS) / 1_000_000) as i32)
+                } else {
+                    None
+                }
+            })),
+            DatePart::Microsecond => Ok(self.unary_opt(|ns| {
+                if (0..NANOSECONDS_IN_DAY).contains(&ns) {
+                    Some(((ns % NANOSECONDS) / 1_000) as i32)
+                } else {
+                    None
+                }
+            })),
+            DatePart::Nanosecond => Ok(self.unary_opt(|ns| {
+                if (0..NANOSECONDS_IN_DAY).contains(&ns) {
+                    Some((ns % NANOSECONDS) as i32)
+                } else {
+                    None
+                }
+            })),

Review Comment:
   The Hour/Minute/Second arms could be deduplicated as they almost identical 
across seconds/milli/micro/nano, but the handling of milli/micro/nano isn't as 
straight forward I guess. Though could be handled with a macro I suppose? Let 
me know if prefer to try reduce this duplication or if it's fine as is



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