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 20:11:34 GMT
Alex Behm has posted comments on this change.

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


Patch Set 2:

(2 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 {
> So now there are basically 2 things we have to do:
Got it, I agree with you. We should fold first.

Btw, while we are at it, let's move this function into the KuduScanNode


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
> Works for me, though there is another caller: HdfsPartitionPruner.canEvalUs
I don't think it "expects" an unanalyzed expr. I think it was just not necessary for that
use case so at the time we did not reset() and analyze. However, to make the behavior of this
function saner it seems we should do that.


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