alamb commented on code in PR #7887:
URL: https://github.com/apache/arrow-rs/pull/7887#discussion_r2198126943


##########
arrow-cast/src/cast/mod.rs:
##########
@@ -891,7 +893,7 @@ pub fn cast_with_options(
                 scale,
                 from_type,
                 to_type,
-                |x: i256| x.to_f64().unwrap(),
+                |x: i256| decimal256_to_f64(x),

Review Comment:
   Do we need to do something similar for Decimal128 above? 



##########
arrow-cast/src/cast/mod.rs:
##########
@@ -1993,6 +1995,17 @@ where
     }
 }
 
+/// Convert a [`i256`] to `f64` saturating to infinity on overflow.
+fn decimal256_to_f64(v: i256) -> f64 {

Review Comment:
   Saturating to INF seems a better solution than panic'ing



##########
arrow-cast/src/cast/mod.rs:
##########
@@ -8660,6 +8673,16 @@ mod tests {
             "did not find expected error '{expected_error}' in actual error 
'{err}'"
         );
     }
+    #[test]
+    fn test_cast_decimal256_to_f64_overflow() {

Review Comment:
   Could you also please either add or ensure there is an existing test for 
casting `Decimal128` (`i128::MIN` and `i128::MAX` to `f64`)



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

Reply via email to