Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16143 )
Change subject: IMPALA-9741: Support querying Iceberg table by impala ...................................................................... Patch Set 20: Code-Review+1 (4 comments) Did another pass, found a few nits, but the code LGTM. Only giving it +1 because Tim mentioned he'll do another round as well. http://gerrit.cloudera.org:8080/#/c/16143/20/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java: http://gerrit.cloudera.org:8080/#/c/16143/20/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@86 PS20, Line 86: notPartitioned = false; nit: probably just call this variable 'partitioned', having a negation in a variable name is confusing a bit. http://gerrit.cloudera.org:8080/#/c/16143/20/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/16143/20/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@134 PS20, Line 134: // LocalFsTable transformed from iceberg table only has one partition nit: then maybe we could use a Preconditions check to verify this. Also, we don't need a for-loop in this case. http://gerrit.cloudera.org:8080/#/c/16143/20/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/16143/20/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@117 PS20, Line 117: 1 In case 1 we cannot filter data files because the column is not part of the partition expressions, right? http://gerrit.cloudera.org:8080/#/c/16143/20/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test: http://gerrit.cloudera.org:8080/#/c/16143/20/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test@4 PS20, Line 4: USE functional_parquet; nit: it can be omitted -- To view, visit http://gerrit.cloudera.org:8080/16143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I856cfee4f3397d1a89cf17650e8d4fbfe1f2b006 Gerrit-Change-Number: 16143 Gerrit-PatchSet: 20 Gerrit-Owner: wangsheng <[email protected]> Gerrit-Reviewer: Anonymous Coward (606) Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Thu, 27 Aug 2020 15:11:19 +0000 Gerrit-HasComments: Yes
