Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24143 )

Change subject: IMPALA-14755: (part 2) Impement Iceberg deletion vector 
reading/writing
......................................................................


Patch Set 1:

(11 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

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

http://gerrit.cloudera.org:8080/#/c/24143/1/be/src/exec/iceberg-delete-builder.cc@186
PS1, Line 186:         ice_del_vector = 
flatbuffers::GetRoot<org::apache::impala::fb::FbIcebergDeletionVector>(
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/be/src/runtime/dml-exec-state.cc
File be/src/runtime/dml-exec-state.cc:

http://gerrit.cloudera.org:8080/#/c/24143/1/be/src/runtime/dml-exec-state.cc@639
PS1, Line 639:   // For puffin files, use the deletion vector record count and 
the total puffin file size.
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/be/src/runtime/dml-exec-state.cc@717
PS1, Line 717:   std::map<std::string, std::pair<const TIcebergDeletionVector*, 
const TIcebergDeletionVector*>>
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/common/fbs/IcebergObjects.fbs
File common/fbs/IcebergObjects.fbs:

PS1:
This file is used in communication between impalad and catalogd/statestore. 
Please make sure impalads can still work with new/old versions of catalogd and 
statestore. Basically only new fields can be added and should be added at the 
end of a table definition.
https://flatbuffers.dev/flatbuffers_guide_writing_schema.html


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:

http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@112
PS1, Line 112:       long snapshotId, boolean isPartitionKeyScan, Map<Hash128, 
TIcebergDeletionVector> dataFileToDV) {
line too long (103 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:

http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@373
PS1, Line 373:     Preconditions.checkState(positionDeletesRecordCount_ != 0 || 
!dataFileToDV_.isEmpty());
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@419
PS1, Line 419:     List<ByteBuffer> addedDeletionVectorsFb = 
icebergOp.getIceberg_added_deletion_vectors_fb();
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@420
PS1, Line 420:     List<ByteBuffer> removedDeletionVectorsFb = 
icebergOp.getIceberg_removed_deletion_vectors_fb();
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@461
PS1, Line 461:         FbIcebergDeletionVector dv = 
FbIcebergDeletionVector.getRootAsFbIcebergDeletionVector(buf);
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@469
PS1, Line 469:         FbIcebergDeletionVector dv = 
FbIcebergDeletionVector.getRootAsFbIcebergDeletionVector(buf);
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:

http://gerrit.cloudera.org:8080/#/c/24143/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@1175
PS1, Line 1175:     else if (cf.format() == FileFormat.PUFFIN) fileFormat = 
FbIcebergDataFileFormat.PUFFIN;
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5613c31a7aa46b94b7c70386c939c08cc68632cd
Gerrit-Change-Number: 24143
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Thu, 26 Mar 2026 13:38:54 +0000
Gerrit-HasComments: Yes

Reply via email to