alamb commented on code in PR #3669:
URL: https://github.com/apache/arrow-datafusion/pull/3669#discussion_r985226627


##########
datafusion/optimizer/src/simplify_expressions.rs:
##########
@@ -790,6 +791,37 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a, 
S> {
                 right,
             } if !info.nullable(&left)? && left == right => lit(1),
 
+            //
+            // Rules for Modulo
+            //
+
+            // A % null --> null
+            BinaryExpr {
+                left: _,
+                op: Modulo,
+                right,
+            } if is_null(&right) => *right,
+            // null % A --> null
+            BinaryExpr {
+                left,
+                op: Modulo,
+                right: _,
+            } if is_null(&left) => *left,
+            // A % 1 --> 0
+            BinaryExpr {
+                left,
+                op: Modulo,
+                right,
+            } if !info.nullable(&left)? && is_one(&right) => lit(0),
+            // A % 0 --> DivideByZero Error
+            BinaryExpr {
+                left,
+                op: Modulo,
+                right,
+            } if !info.nullable(&left)? && is_zero(&right) => {
+                return 
Err(DataFusionError::ArrowError(ArrowError::DivideByZero))

Review Comment:
   I suggest not erroring in this case. It might end up causing a an expression 
like
   
   ```sql
   select 
     CASE 
       WHEN 0 = 0 THEN 5 
       WHEN 0 != 0 THEN 5 % 0
      END
      ...
   ```
   
   In other words, it could generate an error that would not ever actually be 
evaluated at runtime



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