alamb commented on code in PR #7515:
URL: https://github.com/apache/arrow-datafusion/pull/7515#discussion_r1321893464
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -734,19 +744,33 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for
Simplifier<'a, S> {
op: Modulo,
right: _,
}) if is_null(&left) => *left,
- // A % 1 --> 0
+ // A % 1 --> 0 (if A is not nullable and not floating, since NAN %
1 --> NAN)
Expr::BinaryExpr(BinaryExpr {
left,
op: Modulo,
right,
- }) if !info.nullable(&left)? && is_one(&right) => lit(0),
- // A % 0 --> DivideByZero Error
+ }) if !info.nullable(&left)?
+ && !info.get_data_type(&left)?.is_floating()
+ && is_one(&right) =>
+ {
+ lit(0)
+ }
+ // A % 0 --> DivideByZero Error (if A is not floating and not null)
+ // A % 0 --> NAN (if A is floating and not null)
Expr::BinaryExpr(BinaryExpr {
left,
op: Modulo,
right,
}) if !info.nullable(&left)? && is_zero(&right) => {
- return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
+ match info.get_data_type(&left)? {
Review Comment:
I don't really understand the rationale for `float % float` --> `NaN`
(rather than error)
But on the other hand, postgres doesn't seem to support `%` on floating
point values:
```
select 1.0::float % 0::float;
operator does not exist: double precision % double precision LINE 1: select
1.0::float % 0::float; ^ HINT: No operator matches the given name and argument
types. You might need to add explicit type casts.
```
--
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]
