Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/22456 )
Change subject: IMPALA-13739: Part2: Introduce IcebergFileDescriptor ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/22456/3/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java File fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java: http://gerrit.cloudera.org:8080/#/c/22456/3/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@297 PS3, Line 297: hdfs > This is what it is, it's an Hdfs FileDescriptor that we get from the 'hdfsF Ok. http://gerrit.cloudera.org:8080/#/c/22456/3/fe/src/main/java/org/apache/impala/catalog/IcebergFileDescriptor.java File fe/src/main/java/org/apache/impala/catalog/IcebergFileDescriptor.java: http://gerrit.cloudera.org:8080/#/c/22456/3/fe/src/main/java/org/apache/impala/catalog/IcebergFileDescriptor.java@33 PS3, Line 33: FbFileMetadata > Did a quick investigation on this, and apparently it's not that straightfor Why would it be a breaking change? Is it in case of zero-downtime upgrades? I'm ok with doing it later but I think in the long run we should get rid of this extra layer in as many places as possible, especially as its name "FbFileMetadata" is also hard to understand correctly. http://gerrit.cloudera.org:8080/#/c/22456/3/fe/src/main/java/org/apache/impala/catalog/IcebergFileDescriptor.java@90 PS3, Line 90: && > Well, this is existing code, just moved from one place to another. Addition In its previous place this code was in the same class where encoding and decoding were done, so it was easier to understand it there. Here I think the extra info in the comment is worth adding. -- To view, visit http://gerrit.cloudera.org:8080/22456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id08baf4db32197bab74178777c4ba3fec98c2451 Gerrit-Change-Number: 22456 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 11 Feb 2025 12:36:48 +0000 Gerrit-HasComments: Yes
