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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
