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-4213: Planner not pushing some Kudu predicates
Date Wed, 05 Oct 2016 18:56:23 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-4213: Planner not pushing some Kudu predicates
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4613/2/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java:

Line 124:     try {
Do we still need these changes? I kind of liked the previous flow better. I think with the
changes to foldConstantChildren() should be sufficient (we can remove the analyzeNoThrow()
from the original L145 though).


http://gerrit.cloudera.org:8080/#/c/4613/2/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

Line 1195:    * resulting LiteralExpr. Modifies 'this' in place and resets it. Hence, it is
not safe
Seems cleaner to reset() and then analyze() in here. That way callers don't need to worry
about the weird state the modified Expr could be in. It just works.


Line 1213:     reset();
Let's only do this if there was actually folding.


-- 
To view, visit http://gerrit.cloudera.org:8080/4613
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bab22d90ee00a054c847de6c734b4f24a3f5a85
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message