[GitHub] [calcite] kgyrtkirk commented on a change in pull request #2673: [CALCITE-4910] Enhance simplify to reduce ((A OR D) and (A OR C) AND A AND B) to (A AND B) (Ziwei Liu)

[GitBox <gi...@apache.org>]

kgyrtkirk commented on a change in pull request #2673:
URL: https://github.com/apache/calcite/pull/2673#discussion_r778963281



##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -1506,8 +1506,46 @@ RexNode simplifyAnd(RexCall e, RexUnknownAs unknownAs) {
     return simplifyAnd2(terms, notTerms);
   }
 
+  boolean satisfy(Set<RexNode> condition, RexNode term) {
+    List<RexNode> terms = new ArrayList<>();
+    RelOptUtil.decomposeConjunction(term, terms);
+    return condition.containsAll(terms);
+  }
+
+  boolean satisfyForUnknownAsFalse(Set<RexNode> condition, Set<RexNode> notNullOperands,
+      RexNode node) {
+    List<RexNode> terms = new ArrayList<>();
+    RelOptUtil.decomposeConjunction(node, terms);
+    for (RexNode term : terms) {
+      if (!condition.contains(term) && !(term.getKind() == SqlKind.IS_NOT_NULL && notNullOperands
+          .contains(((RexCall) term).getOperands().get(0)))) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  boolean containSatisfyTerm(RexNode node, java.util.function.Predicate<RexNode> satisfy) {
+    List<RexNode> terms = new ArrayList<>();
+    RelOptUtil.decomposeDisjunction(node, terms);
+    for (RexNode term : terms) {
+      if (satisfy.test(term)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   // package-protected only to support a deprecated method; treat as private
   RexNode simplifyAnd2(List<RexNode> terms, List<RexNode> notTerms) {
+    Set<RexNode> termsSet = new HashSet<>(terms);

Review comment:
       I think we already have similar logic here - based on the predicates - and most likely we only miss the handling of an existing predicate somewhere around in the `simplifyIsNull/simpliftIsNotNull` methods
   
   I think this approach is a little bit too much by the way as at first glance I would be afraid that it would make "unsafe" simplifications as well
   
   let me know if you think otherwise - I'll try to take a closer look tomorrow




-- 
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: commits-unsubscribe@calcite.apache.org

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