Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20453 )
Change subject: IMPALA-12413: Make Iceberg tables created by Trino compatible with Impala ...................................................................... Patch Set 1: (3 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/20453/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java: http://gerrit.cloudera.org:8080/#/c/20453/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@250 PS1, Line 250: msTbl.getSd().getInputFormat() > You can replace this with 'inputFormat' Done http://gerrit.cloudera.org:8080/#/c/20453/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@355 PS1, Line 355: FeIcebergTable.setIcebergStorageDescriptor(msTable_); > Could you add a comment for future readers why this is set here? Done http://gerrit.cloudera.org:8080/#/c/20453/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@392 PS1, Line 392: FeIcebergTable.resetIcebergStorageDescriptor(msTable_, msTbl); > What if we let this storage descriptor persist in HMS? Wouldn't it help for It shouldn't hurt setting the storage descriptors (because they should use Impala with a relatively new HMS, and other engines should just ignore these properties), but if they only read the table via Impala it's probably nicer to leave the table as is, so we don't introduce unexpected bugs. If someone modifies the table via Impala, e.g. creates a new snapshot, it's a bit different story, in that case we update the storage descriptors. Setting the storage descriptors / storage handler would help Hive read the table, but I think it shouldn't matter for our implementation. Hive needs to deal with such tables anyway. -- To view, visit http://gerrit.cloudera.org:8080/20453 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18ea3858314d70a6131982a4e4d3ca90a95a311a Gerrit-Change-Number: 20453 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[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, 07 Sep 2023 10:18:31 +0000 Gerrit-HasComments: Yes
