Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23838 )

Change subject: IMPALA-14564: Remove redundant partition info from Iceberg file 
descriptors
......................................................................


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/23838/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23838/11//COMMIT_MSG@19
PS11, Line 19: Since the scan nodes need the partition information for 
execution,
             : we look up the relevant partitions by their ids in the Planner, 
put
             : them in the file descriptors that are sent to the executors.
This means that it is denormalized again when sending to executors, right? Is 
there a plan to make this more efficient in coordinator->executor communication 
too?


http://gerrit.cloudera.org:8080/#/c/23838/11/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/23838/11/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@220
PS11, Line 220: ByteBuffer
I think that in the long term we should replace this with arrays to save more 
memory. There could be followup ticket mentioned in a comment.


http://gerrit.cloudera.org:8080/#/c/23838/11/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@712
PS11, Line 712:    * FlatBuffer content, regardless of how the buffer was 
obtained (e.g. from Thrift's
              :    * fast-path deserialization which may leave position > 0).
org.apache.thrift.TBaseHelper.copyBinary() could be usef for this - it is 
already used at same places in Impala.

https://github.com/apache/thrift/blob/2a93df80f27739ccabb5b885cb12a8dc7595ecdf/lib/java/src/org/apache/thrift/TBaseHelper.java#L272


http://gerrit.cloudera.org:8080/#/c/23838/11/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/23838/11/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@1209
PS11, Line 1209: compressedBb
nit: isn't it more like compactedBb?



--
To view, visit http://gerrit.cloudera.org:8080/23838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57c2fd6f1ebb636aa9e7ca925413ca51858cbc2a
Gerrit-Change-Number: 23838
Gerrit-PatchSet: 11
Gerrit-Owner: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Thu, 21 May 2026 17:11:03 +0000
Gerrit-HasComments: Yes

Reply via email to