[GitHub] [calcite] Aaaaaaron commented on a change in pull request #2253: [CALCITE-4385] Simplifies AND/OR condition that has common expressions, extract and eliminate/merge them as much as possible (Jiatao Tao)

[GitBox]

Aaaaaaron commented on a change in pull request #2253:
URL: https://github.com/apache/calcite/pull/2253#discussion_r521790706



##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -1689,6 +1690,123 @@ RexNode simplifyAnd2ForUnknownAsFalse(List<RexNode> 
terms,
     }
   }
 
+  /**
+   * <p>Simplifies AND/OR condition that has common expressions,
+   * extract and eliminate/merge them as much as possible.</p>
+   * <ul>
+   * <li>(a OR b) AND (a OR b OR c OR d) =&gt; (a OR b)
+   * <li>(a OR b OR c OR d) AND (a OR b OR e OR f) =&gt; (a OR b) OR ((c OR d) 
AND (e OR f))
+
+   * <li>(a AND b) OR (a AND b AND c) =&gt; (a AND b)
+   * <li>(a AND b AND c AND d) OR (a AND b AND e AND f) =&gt;
+   * (a AND b) AND ((c AND d) OR (e AND f))
+   * </ul>
+   * <p> The difference between {@link #simplifyAnd} is that {@link 
#simplifyAnd} mainly
+   * simplifies expressions whose answer can be determined without evaluating 
both sides,
+   * like: FALSE AND (xxx) =&gt; FALSE</p>
+   */
+  public RexNode eliminateCommonExprInCondition(RexNode call) {

Review comment:
       @rubenada Yes, I think this improvement can be a common optimize, so I 
open a sub-task for this, if you see my origin JIRA, you can see the main point 
is in join optimize, I don't want this optimize only attached to join, but the 
first step I want to only used in Join, maybe add this to 
JoinReduceExpressionsRule.
   
   I think the optimize is a kind of simplify, maybe we can merge to 
RexSimplify's main process, like Spark: `BooleanSimplification`, but this 
impact is a bit large, we may need to discuss/watch to control controlling the 
risk. 
   Hope to hear your opinion.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org