Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16143 )
Change subject: IMPALA-9741: Support quering Icebreg table by impala ...................................................................... Patch Set 11: (9 comments) Nice work! Only found some nit comments/grammar mistakes. Now I'm waiting to see some end-to-end tests. Please don't forget to add tests about showing table metadata, e.g., SHOW PARTITIONS, SHOW FILES, etc. http://gerrit.cloudera.org:8080/#/c/16143/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16143/11//COMMIT_MSG@7 PS11, Line 7: quering Icebreg misspelled querying Also, still misspelled Iceberg http://gerrit.cloudera.org:8080/#/c/16143/11//COMMIT_MSG@9 PS11, Line 9: query nit: querying http://gerrit.cloudera.org:8080/#/c/16143/11//COMMIT_MSG@30 PS11, Line 30: query querying http://gerrit.cloudera.org:8080/#/c/16143/11//COMMIT_MSG@31 PS11, Line 31: decided decide http://gerrit.cloudera.org:8080/#/c/16143/11//COMMIT_MSG@32 PS11, Line 32: transformed these transfer this http://gerrit.cloudera.org:8080/#/c/16143/11/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/16143/11/common/thrift/CatalogObjects.thrift@516 PS11, Line 516: s nit: Plural 's' not needed since the value type of the map is not a list http://gerrit.cloudera.org:8080/#/c/16143/11/common/thrift/CatalogObjects.thrift@516 PS11, Line 516: 5: optional map<string,THdfsFileDesc> path_md5_to_file_descriptors nit: please add comment about field 5. http://gerrit.cloudera.org:8080/#/c/16143/6/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/6/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@88 PS6, Line 88: if (table_ instanceof FeIcebergTable) { : if (((FeIcebergTable) table_).getSourceIdToPartitionMap().isEmpty()) { : notPartitioned = true; : } > Yes, you are right, we treated iceberg table as unpartitioned hdfs table, b I see, thanks for the explanation. http://gerrit.cloudera.org:8080/#/c/16143/11/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/16143/11/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@258 PS11, Line 258: PartitionColToSourceId SourceIdToPartition -- 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: 11 Gerrit-Owner: wangsheng <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Fri, 17 Jul 2020 12:07:00 +0000 Gerrit-HasComments: Yes
