Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/20515 )
Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables ...................................................................... Patch Set 11: (6 comments) Thank you, Gabor and Tamas! http://gerrit.cloudera.org:8080/#/c/20515/9/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/20515/9/common/thrift/CatalogObjects.thrift@634 PS9, Line 634: // Indicates whether the request could be exchanged with a truncate command > required? Done http://gerrit.cloudera.org:8080/#/c/20515/9/fe/src/main/java/org/apache/impala/analysis/PartitionSet.java File fe/src/main/java/org/apache/impala/analysis/PartitionSet.java: http://gerrit.cloudera.org:8080/#/c/20515/9/fe/src/main/java/org/apache/impala/analysis/PartitionSet.java@57 PS9, Line 57: List<Expr> conjuncts = new ArrayList<>(); > nit: same line as 'if' Done http://gerrit.cloudera.org:8080/#/c/20515/9/fe/src/main/java/org/apache/impala/analysis/PartitionSet.java@93 PS9, Line 93: } > No brackets are needed. Done http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java: http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@253 PS8, Line 253: String.format("Unable to parse timestamp value from: %s", > Probably a misclick and wanted to comment eh LOG.warn() lines. Done http://gerrit.cloudera.org:8080/#/c/20515/9/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/20515/9/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@241 PS9, Line 241: /** > I think this is an optional member. Should check isset first to be on the s Done http://gerrit.cloudera.org:8080/#/c/20515/9/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@246 PS9, Line 246: DeleteFiles deleteFiles = iceTxn.newDelete(); > nit: merge with line above Done -- To view, visit http://gerrit.cloudera.org:8080/20515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9 Gerrit-Change-Number: 20515 Gerrit-PatchSet: 11 Gerrit-Owner: Peter Rozsa <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 22 Nov 2023 15:10:06 +0000 Gerrit-HasComments: Yes
