Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16721 )
Change subject: IMPALA-10152: Add support for Iceberg HiveCatalog ...................................................................... Patch Set 6: (3 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/16721/4/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/16721/4/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@419 PS4, Line 419: cebergTable.METAD > This table property is generated by HiveCatalog, I'm curious about: Good point! 'metadata_location' is internal to Iceberg so we shouldn't allow users modifying it. Updated the code accordingly. http://gerrit.cloudera.org:8080/#/c/16721/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java: http://gerrit.cloudera.org:8080/#/c/16721/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java@65 PS4, Line 65: return hiveCatalog_.createTable(identifier, schema, spec, location, properties); : } > nits: One line is ok, unnecessary for two lines. Done http://gerrit.cloudera.org:8080/#/c/16721/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/16721/4/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@242 PS4, Line 242: * Get TIcebergFileFormat from a string, usually from table properties. : * > Maybe we need add a comment here, since we change the default value to 'PAR Updated the code a bit. Now this method is returning PARQUET when 'format' is null. Format can be null when the table was created by other engines. And returning null when the format string is invalid. -- To view, visit http://gerrit.cloudera.org:8080/16721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie574589a1751aaa9ccbd34a89c6819714d103197 Gerrit-Change-Number: 16721 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[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, 20 Nov 2020 09:45:15 +0000 Gerrit-HasComments: Yes
