Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20903 )
Change subject: IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables ...................................................................... Patch Set 2: Code-Review+2 (4 comments) Thank you for the update Zoltan! Marked some nits, but LGTM! http://gerrit.cloudera.org:8080/#/c/20903/2/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/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@361 PS2, Line 361: public nit: this could be private for now http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@373 PS2, Line 373: public nit: this could be private for now http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@384 PS2, Line 384: nit: indentation http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@393 PS2, Line 393: rowDelta.validateFromSnapshot(initialSnapshotId); : rowDelta.validateNoConflictingDataFiles(); : rowDelta.commit(); nit: this could be chained -- To view, visit http://gerrit.cloudera.org:8080/20903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2ceb80b939c644388707b21061bf55451234dcd3 Gerrit-Change-Number: 20903 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Noemi Pap-Takacs <npaptak...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 18 Jan 2024 15:35:44 +0000 Gerrit-HasComments: Yes