Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/22605 )
Change subject: IMPALA-13738 (Part1): Denormalize IcebergTable.getPartialInfo() ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/22605/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22605/1//COMMIT_MSG@10 PS1, Line 10: removes the dependency on the internal hdfsTable_ Does this patch already remove the dependency or just reduce it? http://gerrit.cloudera.org:8080/#/c/22605/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/22605/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@774 PS1, Line 774: wantPartitionInfo In the previous code, these infos were only given if "partIds != null", it it intentional that now also 'wantPartitionInfo' enables it? http://gerrit.cloudera.org:8080/#/c/22605/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@800 PS1, Line 800: -1 Is it intentional that this is changed from part.getLastCompactionId()? http://gerrit.cloudera.org:8080/#/c/22605/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@838 PS1, Line 838: table_info Can this be NULL? We've already set some of its fields earlier. We could convert it into a Preconditions check. -- To view, visit http://gerrit.cloudera.org:8080/22605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75d277fb555d0088a2e28111ff529a4605d734fa Gerrit-Change-Number: 22605 Gerrit-PatchSet: 1 Gerrit-Owner: Noemi Pap-Takacs <npaptak...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Tue, 11 Mar 2025 12:34:18 +0000 Gerrit-HasComments: Yes
