Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24572 )
Change subject: IMPALA-15171: Null out Iceberg delete file path slot after the join ...................................................................... Patch Set 6: (5 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/24572/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24572/5//COMMIT_MSG@9 PS5, Line 9: IcebergScanPlanner materializes the INPUT__FILE__NAME (file path) > I have a basic question about design: why do we even use INPUT__FILE__NAME? Using INPUT__FILE__NAME in the Scan and Delete operator's fragment is quite cheap as the cost amortizes between lots of rows. Adding an extra INPUT__FILE__NAME->INPUT__FILE__ID mapping and distributing among executors would add some extra overhead and complexities. So I'm not sure it's worth doing, but feel free to file a ticket for this. I think adding a SelectNode between the Delete node and the Union node, to eliminate the extra slots, would be the cleanest way forward. But it's also more complicated than the current patch, and might introduce perf regression in some cases. For pure Iceberg V3 tables with Deletion Vectors only we could push down Deletion Vectors to the Scan operator. I think this would give us the best performance. I filed a ticket about it a few months ago: IMPALA-15041 http://gerrit.cloudera.org:8080/#/c/24572/5//COMMIT_MSG@10 PS5, Line 10: position-delete join : (IcebergDeleteJoinNode) can use it as a join key. > Is the issue still relevant with Iceberg v3, or only v2 tables are affected Relevant for V2 and V3. Deletion Vectors still mark delete records based on their position. http://gerrit.cloudera.org:8080/#/c/24572/5/be/src/exec/iceberg-delete-node.cc File be/src/exec/iceberg-delete-node.cc: http://gerrit.cloudera.org:8080/#/c/24572/5/be/src/exec/iceberg-delete-node.cc@273 PS5, Line 273: > nit: Are these "See IMPALA-15171" tags needed everywhere? I removed these from a few places, kept at variable declarations. http://gerrit.cloudera.org:8080/#/c/24572/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/24572/5/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@168 PS5, Line 168: private boolean clearFilePathSlot_ = false; > This field should be added to the plan for easier identification Done http://gerrit.cloudera.org:8080/#/c/24572/5/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/24572/5/tests/query_test/test_iceberg.py@2907 PS5, Line 2907: n{2}".format > nit: not needed Done -- To view, visit http://gerrit.cloudera.org:8080/24572 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e662cb6a98dd3e687185d384731d6be45f91b94 Gerrit-Change-Number: 24572 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 30 Jul 2026 13:08:00 +0000 Gerrit-HasComments: Yes
