Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8480 )
Change subject: IMPALA-4985: use parquet stats of nested types for dynamic pruning ...................................................................... Patch Set 6: (5 comments) lgtm after final nits http://gerrit.cloudera.org:8080/#/c/8480/6/fe/src/main/java/org/apache/impala/analysis/SlotRef.java File fe/src/main/java/org/apache/impala/analysis/SlotRef.java: http://gerrit.cloudera.org:8080/#/c/8480/6/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@154 PS6, Line 154: * Checks if slotRef refers to an array "pos" pseudo-column. Checks if this SlotRef http://gerrit.cloudera.org:8080/#/c/8480/6/fe/src/main/java/org/apache/impala/analysis/SlotRef.java@160 PS6, Line 160: public boolean isArrayPosReference() { isArrayPosRef() (we typically abbreviate Reference with Ref) http://gerrit.cloudera.org:8080/#/c/8480/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/8480/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@439 PS5, Line 439: Preconditions.checkState(slotRef.getDesc().isScanSlot()); > ah, didn't understand this as "move this method to the SlotRef class". Done Sorry! I realize my comment was not clear. Thanks for addressing. http://gerrit.cloudera.org:8080/#/c/8480/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/8480/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@541 PS6, Line 541: // It is assumed that analysis adds these guards such that they are correct, but guards -> filters http://gerrit.cloudera.org:8080/#/c/8480/5/testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test File testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test: http://gerrit.cloudera.org:8080/#/c/8480/5/testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test@96 PS5, Line 96: ---- PLAN > that's the lowest level (at the moment) that prints out "parquet statistics Got it. Might want to change that, but definitely not in this patch. The motivation for changing is that dealing with conflicts in planner tests is becoming increasingly painful. -- To view, visit http://gerrit.cloudera.org:8080/8480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe Gerrit-Change-Number: 8480 Gerrit-PatchSet: 6 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 21 Nov 2017 23:57:17 +0000 Gerrit-HasComments: Yes
