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

Change subject: IMPALA-13088: (part 1) Improve build batch processing of 
IcebergDeleteBuilder
......................................................................


Patch Set 3:

(6 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.h
File be/src/exec/iceberg-delete-builder.h:

http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.h@95
PS2, Line 95:   // Collects the processed data files' file paths and fills 
'intermediate_delete_rows_'
> The function comment is not valid now. Would be nice to mention that this p
Done


http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.h@124
PS2, Line 124:   // Keys are data file paths processed by the join operator. 
Values are the file
> Can you add a comment what the keys and values are for in this map?
Done


http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.h@146
PS2, Line 146:
             :   /// Inserts the conte
> nit: seems a weird place to break the line
Done


http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.cc
File be/src/exec/iceberg-delete-builder.cc:

http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.cc@184
PS2, Line 184: auto& delete_vector =
> To increase readability, do you think it'd make sense to introduce a variab
Done


http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.cc@259
PS2, Line 259: rrent delet
> nit: I'd break the line before the first param, and then each param could f
Done


http://gerrit.cloudera.org:8080/#/c/21435/2/be/src/exec/iceberg-delete-builder.cc@325
PS2, Line 325:       state->LogError(
> KrpcDataStreamSender already filters out NULL file_paths. Is there a need t
We can see NULLs when NUM_NODES=1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14541a064a522d4780fb5f02636736259e79b9cf
Gerrit-Change-Number: 21435
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Jun 2024 16:31:29 +0000
Gerrit-HasComments: Yes

Reply via email to