Alex Behm has posted comments on this change. Change subject: IMPALA-4571: Push IN predicates to Kudu ......................................................................
Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/5316/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: Line 371: > this is passed into the fn on line 380 yes, but the fn doesn't need the type because you can e.getType() http://gerrit.cloudera.org:8080/#/c/5316/3/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: Line 370: PrimitiveType type = ref.getType().getPrimitiveType(); not needed (use e.getType() in getKuduInListValue()) Line 375: if (!(predicate.getChild(i) instanceof LiteralExpr)) return false; predicate,getChild(i).isLiteral() Line 377: // Get the value as an Object. remove this comment and blank line above, code seems self-explanatory Line 395: private static Object getKuduInListValue(PrimitiveType type, Expr e) { pass a LiteralExpr, and make the caller case Line 396: Preconditions.checkArgument(e instanceof LiteralExpr); not really needed if you address above comments Line 397: switch (type) { use e.getType() -- To view, visit http://gerrit.cloudera.org:8080/5316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-HasComments: Yes