Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@10 PS2, Line 10: DISTRIBUTED BROADCAST http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@13 PS2, Line 13: we don't need to update the : IcebergDeleteState which tracks the state of the merge join. we rely on the previous delete updating IcebergDeleteState to the next deleted row id and skip the binary search if it's greater than or equal to the current probe row id. http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@22 PS2, Line 22: getting being http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@24 PS2, Line 24: reset It's not really a reset, maybe: This patch adds a fix to ignore presumptions and do a binary search when the ... http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc File be/src/exec/iceberg-delete-node.cc: http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc@332 PS2, Line 332: int64_t step = current_probe_pos_ - *probe_pos; nit: could be const http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc@348 PS2, Line 348: So we are always doing a binary : // search here to find the proper delete row id. We could improve this case if we store the deleted row ids in an std::unordered_set too and switching to use that at the first case of step!=1 for O(1) lookups at the cost of slightly more memory. We could create a followup jira too, or just skip this and focus on directed distribution mode. http://gerrit.cloudera.org:8080/#/c/20295/2/testdata/datasets/functional/functional_schema_template.sql File testdata/datasets/functional/functional_schema_template.sql: http://gerrit.cloudera.org:8080/#/c/20295/2/testdata/datasets/functional/functional_schema_template.sql@3701 PS2, Line 3701: iceberg_lineitem_multiblock I meant to test this case with iceberg_lineitem_multiblock, seems like I messed up something during creation, we might as well delete this -- To view, visit http://gerrit.cloudera.org:8080/20295 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924 Gerrit-Change-Number: 20295 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Comment-Date: Tue, 01 Aug 2023 15:39:47 +0000 Gerrit-HasComments: Yes
