stuartcarnie commented on code in PR #2251:
URL: https://github.com/apache/arrow-rs/pull/2251#discussion_r934957161


##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -1584,6 +1625,303 @@ fn cast_string_to_date64<Offset: OffsetSizeTrait>(
     Ok(Arc::new(array) as ArrayRef)
 }
 
+fn seconds_since_midnight(time: &chrono::NaiveTime) -> i32 {
+    let sec = time.num_seconds_from_midnight();
+    let frac = time.nanosecond();
+    let adjust = if frac < 1_000_000_000 { 0 } else { 1 };
+    (sec + adjust) as i32
+}
+
+fn milliseconds_since_midnight(time: &chrono::NaiveTime) -> i32 {
+    /// The number of nanoseconds per millisecond.
+    const NANOS_PER_MILLI: u32 = 1_000_000;
+    /// The number of milliseconds per second.
+    const MILLIS_PER_SEC: u32 = 1_000;
+
+    let sec = time.num_seconds_from_midnight() * MILLIS_PER_SEC;
+    let frac = time.nanosecond();
+    let (frac, adjust) = if frac < 1_000_000_000 {
+        (frac, 0)
+    } else {
+        (frac - 1_000_000_000, MILLIS_PER_SEC)
+    };
+    (sec + adjust + frac / NANOS_PER_MILLI) as i32
+}
+
+/// Casts generic string arrays to `Time32SecondArray`
+fn cast_string_to_time32second<Offset: OffsetSizeTrait>(
+    array: &dyn Array,
+    cast_options: &CastOptions,
+) -> Result<ArrayRef> {
+    let string_array = array
+        .as_any()
+        .downcast_ref::<GenericStringArray<Offset>>()
+        .unwrap();
+
+    let array = if cast_options.safe {
+        let iter = (0..string_array.len()).map(|i| {
+            if string_array.is_null(i) {
+                None
+            } else {
+                string_array
+                    .value(i)
+                    .parse::<chrono::NaiveTime>()
+                    .map(|time| seconds_since_midnight(&time))
+                    .ok()
+            }
+        });

Review Comment:
   I tried, but unfortunately it fails at runtime with a panic:
   
   ```
   thread 'compute::kernels::cast::tests::test_cast_string_to_time32second' 
panicked at 'trusted_len_unzip requires an upper limit', 
arrow/src/array/array_primitive.rs:470:25
   ```
   
   at
   
   
https://github.com/apache/arrow-rs/blob/91b2d7e66b066d54035774625b29ff226c755172/arrow/src/array/array_primitive.rs#L469-L470
   
   The previous `iter` was ultimately of type `Range<usize>`, which returns 
`Some(_)` for the `size_hint`:
   
   
https://github.com/rust-lang/rust/blob/fe3342816a282949f014caa05ea2e669ff9d3d3c/library/core/src/iter/range.rs#L714-L722
   
   whereas our new `iter` does not, as it does not know the size. 
   
   There are probably good reasons, but it is unfortunate that `size_hint` was 
not a separate trait, so this could be caught at compile time.



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