Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18658 )
Change subject: IMPALA-11287 (part 2): Implement CREATE TABLE LIKE for Iceberg tables ...................................................................... Patch Set 4: (4 comments) Thank you for working on this! Could you please document this new feature in docs/topics/impala_iceberg.xml ? http://gerrit.cloudera.org:8080/#/c/18658/4/common/thrift/JniCatalog.thrift File common/thrift/JniCatalog.thrift: http://gerrit.cloudera.org:8080/#/c/18658/4/common/thrift/JniCatalog.thrift@514 PS4, Line 514: required I think this sholud be optional as we haven't use this field earlier. Also, can't we just use the source table at the Catalog-side to retrieve the columns? http://gerrit.cloudera.org:8080/#/c/18658/4/common/thrift/JniCatalog.thrift@516 PS4, Line 516: // Current PartitionSpec of Iceberg tables : 13: optional CatalogObjects.TIcebergPartitionSpec partition_spec : : // Map of string properties for Iceberg tables : 14: optional map<string, string> table_properties Can't we retrieve this information at the Catalog side from the source table? http://gerrit.cloudera.org:8080/#/c/18658/4/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java: http://gerrit.cloudera.org:8080/#/c/18658/4/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@215 PS4, Line 215: IcebergTable It's better to use FeIcebergTable, because if local catalog mode is used then this is a LocalIcebergTable. Though I think the contents of the if-stmt could be put at the Catalog-side, where we only have IcebergTable. http://gerrit.cloudera.org:8080/#/c/18658/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java File fe/src/main/java/org/apache/impala/util/IcebergUtil.java: http://gerrit.cloudera.org:8080/#/c/18658/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@194 PS4, Line 194: ) nit: or partitionFields.empty() ? -- To view, visit http://gerrit.cloudera.org:8080/18658 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1284b926f51158e221277b18b2e73707e29f86ac Gerrit-Change-Number: 18658 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 30 Jun 2022 17:24:55 +0000 Gerrit-HasComments: Yes
