Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17466 )
Change subject: IMPALA-10626: Add support for Iceberg's Catalogs API ...................................................................... Patch Set 5: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/17466/5/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: http://gerrit.cloudera.org:8080/#/c/17466/5/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@680 PS5, Line 680: putGeneratedKuduProperty This name could be changed as we are using this function for non Kudu stuff too. http://gerrit.cloudera.org:8080/#/c/17466/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogs.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogs.java: http://gerrit.cloudera.org:8080/#/c/17466/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogs.java@72 PS5, Line 72: equals Maybe it would be safer to use case insensitive comparison. http://gerrit.cloudera.org:8080/#/c/17466/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogs.java@163 PS5, Line 163: JNDI uses the context class loader It would be nice to create an Iceberg issue for this, as other users of the API may also hit this. My understanding is that libraries should not rely on contextClassLoader and add a ClassLoader argument to the public functions that need one. -- To view, visit http://gerrit.cloudera.org:8080/17466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5dfa150986117fc55b28034c4eda38a736460ead Gerrit-Change-Number: 17466 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Tue, 13 Jul 2021 00:07:00 +0000 Gerrit-HasComments: Yes
