Alex Behm has posted comments on this change. Change subject: IMPALA-5602: Fix query optimization for kudu and datasource tables ......................................................................
Patch Set 8: (9 comments) Nice! Looks much better. Minor nits left http://gerrit.cloudera.org:8080/#/c/7560/8/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: PS8, Line 444: public void loadSchemaFromKudu() throws ImpalaRuntimeException > Extracted relevant code from KuduTable.load() for loading metadata from kud Move this above loadSchema() so we have these functions clustered. http://gerrit.cloudera.org:8080/#/c/7560/8/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java File fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java: Line 513: public boolean hasStorageLayerConjuncts() { single line? (and elsewhere) http://gerrit.cloudera.org:8080/#/c/7560/8/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: Line 526: public boolean hasStorageLayerConjuncts() { single line? http://gerrit.cloudera.org:8080/#/c/7560/8/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: Line 217: * Returns true if the node contains any conjuncts to be processed by Impala Returns true if this node has conjuncts to be evaluated by Impala against the scan tuple. (This is more precise, because it excludes conjuncts on nested collections as well as dictionary and/or min/max filters which are also evaluated by Impala) Line 224: * Returns true if the node contains any conjuncts pushed to the underlying storage Returns true if this node has conjuncts to be evaluated by the underlying storage engine. Line 228: // Derived classes must override this method if they have any pushed conjuncts don't think we need this, then single line http://gerrit.cloudera.org:8080/#/c/7560/8/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java: Line 169: * Add a new dummy HDFS or external Kudu table to the catalog based on the given CREATE Remove the "HDFS or external Kudu" part here. You already explain the limitations later (which seems more suitable to me) Line 199: }else if (dummyTable instanceof KuduTable) { space after "}" http://gerrit.cloudera.org:8080/#/c/7560/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: Line 318: addTestTable("CREATE EXTERNAL TABLE kudu_planner_test.no_stats STORED AS KUDU " + This is pretty awesome, thanks for adding it! -- To view, visit http://gerrit.cloudera.org:8080/7560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93822d67ebda41d5d0456095c429e3915a3f40c4 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
