Peter Rozsa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22761 )

Change subject: IMPALA-13932: Add file path and position-based duplicate check 
for IcebergMergeNode
......................................................................


Patch Set 2:

(5 comments)

Thanks, Zoltan!

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

http://gerrit.cloudera.org:8080/#/c/22761/1//COMMIT_MSG@8
PS1, Line 8:
> nit: title should be single line
Done


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

http://gerrit.cloudera.org:8080/#/c/22761/1/be/src/exec/iceberg-merge-node.cc@187
PS1, Line 187:         // Materialize it as a string
             :         if 
(std::holds_alternative<std::string_view>(previous_row_file_path_)) {
             :           auto* view = 
std::get_if<std::string_view>(&previous_row_file_path_);
> Would it be the same as?
Unfortunately, the autoconversion will not work this way, I used 
holds_alternative and get_if to achieve it.


http://gerrit.cloudera.org:8080/#/c/22761/1/be/src/exec/iceberg-merge-node.cc@293
PS1, Line 293:   auto file_path_sv = 
delete_meta_evaluators_[0]->GetStringVal(actual_row);
             :   auto file_pos = 
delete_meta_evaluators_[1]->GetBigIntVal(actual_row);
             :   i
> nit: fits single line
Done


http://gerrit.cloudera.org:8080/#/c/22761/1/be/src/exec/iceberg-merge-node.cc@296
PS1, Line 296:
> nit: underscore is not needed.
Done


http://gerrit.cloudera.org:8080/#/c/22761/1/be/src/exec/iceberg-merge-node.cc@303
PS1, Line 303: }
             :
             : void IcebergMergeNode::SavePreviousRowPtrAndIdent(TupleRow* 
actual_row) {
             :   impala_udf::
> nit: return !different;
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I71b47414321675958c05438ef3aeeb5df0128033
Gerrit-Change-Number: 22761
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pro...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pro...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 15:56:33 +0000
Gerrit-HasComments: Yes

Reply via email to