Alex Behm has posted comments on this change. Change subject: IMPALA-5280: Coalesce chains of OR conditions to an IN predicate ......................................................................
Patch Set 4: (4 comments) We're almost done here, nice work! http://gerrit.cloudera.org:8080/#/c/7110/4/fe/src/main/java/org/apache/impala/rewrite/EqualityDisjunctsToInRule.java File fe/src/main/java/org/apache/impala/rewrite/EqualityDisjunctsToInRule.java: Line 30: * Coalesce disjunctive equality predicates to an IN predicate, Coalesces ... IN predicate, and ... Line 57: * The transformation is applied, if one of the children is an IN predicate and the other child remove first comma Line 80: } else else if http://gerrit.cloudera.org:8080/#/c/7110/3/fe/src/main/java/org/apache/impala/rewrite/NormalizeBinaryPredicatesRule.java File fe/src/main/java/org/apache/impala/rewrite/NormalizeBinaryPredicatesRule.java: Line 43: if (!expr.getChild(0).isConstant() || expr.getChild(1).isConstant()) { > Personally, I prefer a flow which is easy to understand, even at the expens Fair point about the logic being more comprehensible. To make us both happy how about you add helper functions for the cases, e.g.: boolean isExprOpConstant(Expr e) { return !e.getChild(0).isConstant() && e.getChild(1).isConstant(); } boolean isExprOpSlotRef() { return expr.getChild(0).unwrapSlotRef(false) == null && expr.getChild(1).unwrapSlotRef(false) != null; } if (isExprOpSlotRef(expr) || isExprOpConstant()) { // Do the trnasofrmation } -- To view, visit http://gerrit.cloudera.org:8080/7110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If02396b752c5497de9a92828c24c8062027dc2e2 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: sandeep akinapelli <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: sandeep akinapelli <[email protected]> Gerrit-HasComments: Yes
