tustvold commented on code in PR #4465:
URL: https://github.com/apache/arrow-rs/pull/4465#discussion_r1251959550
##########
arrow-arith/src/arithmetic.rs:
##########
@@ -2584,11 +2026,11 @@ mod tests {
}
#[test]
- #[should_panic(expected = "DivideByZero")]
fn test_f32_array_modulus_dyn_by_zero() {
let a = Float32Array::from(vec![1.5]);
let b = Float32Array::from(vec![0.0]);
- modulus_dyn(&a, &b).unwrap();
+ let result = modulus_dyn(&a, &b).unwrap();
+ assert!(result.as_primitive::<Float32Type>().value(0).is_nan());
Review Comment:
Floating point arithmetic now follows the IEEE 754 standard. My research
showed databases to handle division by zero very inconsistently, some returning
null and some an error. Broadly speaking it seems peculiar to special case
division by zero, and not any of the [other
cases](https://en.wikipedia.org/wiki/NaN#Operations_generating_NaN) that can
lead to `Nan`. Much like we do for total ordering of floats, I think we should
just follow the floating point standard rather than trying to copy some subset
of the databases in the wild. As a side benefit this is also significantly
faster :smile:
--
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]
