You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2024/09/04 09:47:24 UTC

[Impala-ASF-CR] IMPALA-13325: Use RowBatch::CopyRows in IcebergDeleteNode

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

Change subject: IMPALA-13325: Use RowBatch::CopyRows in IcebergDeleteNode
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21718/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21718/1//COMMIT_MSG@18
PS1, Line 18: Time spent in IcebergDeleteNode operator  was between 16-18 seconds
extra space


http://gerrit.cloudera.org:8080/#/c/21718/1/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:

http://gerrit.cloudera.org:8080/#/c/21718/1/be/src/exec/iceberg-delete-node.cc@300
PS1, Line 300:   while (!probe_iterator->AtEnd() && rows_to_copy < remaining_capacity) {
             :     TupleRow* current_probe_row = probe_iterator->Get();
             :     probe_iterator->Next();
             :     uint64_t current_probe_pos = std::make_unsigned_t<int64_t>(
             :         *current_probe_row->GetTuple(0)->GetBigIntSlot(pos_offset_));
Do we need to do this one by one? If current_probe_pos increases monotonically, then we could lookup the next deleted value in deletes and jump to there.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46487fefa300027e9df6cd7fb36c78af01dd56c1
Gerrit-Change-Number: 21718
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 09:47:24 +0000
Gerrit-HasComments: Yes