Gabor Kaszab 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 4:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc@332
PS4, Line 332: current_probe_pos_ - *probe_pos
If I understand what 'current_probe_pos_' and '*probe_pos' means I think this 
should be '*probe_pos - current_probe_pos' instead.


http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc@332
PS4, Line 332:   const int64_t step = current_probe_pos_ - *probe_pos;
This 'step' is only used for checking if we process the next row ID that is 
right after the one we last processed, right?
If yes, then this can be a bool, I think.

e.g.
bool isConsecutivePos = current_probe_pos == 1 + *probe_pos;
Not sure about the name of the variable, though.


http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:

http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/bin/create-load-data.sh@436
PS4, Line 436:   # IMPALA-12327: Add multiblock test for Iceberg tables.
Just wondering if this step could be added to functional_schema_template.sql? I 
do see hadoop mkdir and command there as well.


http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql@3719
PS4, Line 3719: LIKE PARQUET 
'/test-warehouse/lineitem_sixblocks_iceberg/lineitem_sixblocks.parquet'
Is there a way to load the necessary file into this location without running 
create-load-data? Or is there a way to run create-load-data that only runs the 
custom-pre-load-steps that is needed for this table?


http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql@3722
PS4, Line 3722: ALTER TABLE {db_name}{db_suffix}.{table_name} CONVERT TO 
ICEBERG;
Nice! The branch new CONVERT TO ICEBERG in action :)



--
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: 4
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-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 02 Aug 2023 08:34:08 +0000
Gerrit-HasComments: Yes

Reply via email to