Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying ...................................................................... Patch Set 5: (6 comments) Did an initial look, found some minor issues, but overall looks good! http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG@12 PS5, Line 12: statment typo: statement http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG@22 PS5, Line 22: NotImplementedExpception typo: NotImplementedException http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@876 PS5, Line 876: if (table instanceof IcebergMetadataTable) : return new IcebergMetadataTableRef(tableRef, resolvedPath); nit: multi-line if statements should use brackets http://gerrit.cloudera.org:8080/#/c/19483/5/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/19483/5/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@417 PS5, Line 417: ); We could also pass 'e' as the second parameter so we'd get more context when an exception is thrown. http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java: http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@39 PS5, Line 39: metadtata typo: metadata http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2215 PS5, Line 2215: metadtata typo: metadata, and probably write "metadata table" -- To view, visit http://gerrit.cloudera.org:8080/19483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b Gerrit-Change-Number: 19483 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 23 Feb 2023 16:58:27 +0000 Gerrit-HasComments: Yes
