Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23800 )
Change subject: IMPALA-14555: Add Iceberg support for SHOW PARTITIONS WHERE ...................................................................... Patch Set 3: (11 comments) Thanks Peter, the code has simplified a lot thanks to the reviews! http://gerrit.cloudera.org:8080/#/c/23800/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23800/2//COMMIT_MSG@33 PS2, Line 33: > Some code parts look AI-generated. Please include 'Generated-by:' tag: http Done http://gerrit.cloudera.org:8080/#/c/23800/2/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/23800/2/common/thrift/Frontend.thrift@257 PS2, Line 257: // Only used for HDFS tables. > nit: this comment is a bit odd, maybe just "Only used for HDFS tables"? Done http://gerrit.cloudera.org:8080/#/c/23800/2/common/thrift/Frontend.thrift@260 PS2, Line 260: Iceberg > nit: Iceberg Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionStatsHelper.java File fe/src/main/java/org/apache/impala/analysis/IcebergPartitionStatsHelper.java: http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionStatsHelper.java@32 PS2, Line 32: > I think this class is not really needed; the computeFilteredPartitionStats Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionStatsHelper.java@72 PS2, Line 72: > Please add BETWEEN rewriting to IcebergPartitionExpressionRewriter instead Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionStatsHelper.java@97 PS2, Line 97: : : : : : : : : : > This check should be pushed down to getPartitionStats instead, where the Ic Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionStatsHelper.java@108 PS2, Line 108: > This comment seems a bit odd; there's no filtering API called here, the pre Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java: http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@55 PS2, Line 55: // Optional WHERE predicate for SHOW PARTITIONS. > nit: it sounds a bit artificial, consider: "Supported for HDFS and Iceberg Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@525 PS2, Line 525: > nit: Iceberg table Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@598 PS2, Line 598: if (filterExpr == null) { > Please use IcebergUtil.planFiles Done http://gerrit.cloudera.org:8080/#/c/23800/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@1039 PS2, Line 1039: fieldNameToPartitionValue.put(spec.fields().get(i).name(), partValueString); > Is this required? getPartitionKey is only used inside this interface, you c Thanks for pointing this out, removed it. -- To view, visit http://gerrit.cloudera.org:8080/23800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c0ee4d171ae939770725d89dc504e13f82a7688 Gerrit-Change-Number: 23800 Gerrit-PatchSet: 3 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Vanko <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 08 Jan 2026 08:59:34 +0000 Gerrit-HasComments: Yes
