impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5280: Coalesce chains of OR conditions to an IN predicate
Date Tue, 13 Jun 2017 23:31:52 GMT
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 <sakinapelli@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sakinapelli@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message