Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20973 )

Change subject: IMPALA-12765: Balance consecutive partitions better for Iceberg 
tables
......................................................................


Patch Set 3:

(5 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/20973/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20973/2//COMMIT_MSG@25
PS2, Line 25: e
> Nit: chance.
Done


http://gerrit.cloudera.org:8080/#/c/20973/2//COMMIT_MSG@27
PS2, Line 27: With this patch, IcebergScanNode orders its file descriptors 
based on
> Could you elaborate why it i beneficial to assign neighbouring partitions t
Added some details and examples.


http://gerrit.cloudera.org:8080/#/c/20973/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20973/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@55
PS1, Line 55: // List of files need to be scanned by t
> It is only sorted if the table is partitioned, isn't it?
Currently yes, because there's no need to sort ranges of unpartitioned tables. 
OTOH, that might wouldn't add too much overhead, and the code would become 
simpler.


http://gerrit.cloudera.org:8080/#/c/20973/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@210
PS1, Line 210: verride
             :   protected Map<Long, List<FileDe
> It is only sorted if the table is partitioned, isn't it?
Yes, I added a condition to the sort.


http://gerrit.cloudera.org:8080/#/c/20973/2/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/20973/2/tests/query_test/test_iceberg.py@1086
PS2, Line 1086:       for files_rejected_str in files_rejected_array:
> Optional: I find 'continue' to be a bit more difficult to follow than a con
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60773965ecbb4d8e659db158f1f0ac76086d5578
Gerrit-Change-Number: 20973
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 30 Jan 2024 13:00:57 +0000
Gerrit-HasComments: Yes

Reply via email to