Peter Rozsa has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23455 )

Change subject: IMPALA-14108: Add support for SHOW FILES IN table PARTITION for 
Iceberg tables
......................................................................


Patch Set 5: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/23455/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23455/3//COMMIT_MSG@25
PS3, Line 25: Testing:
> Added some tests, it takes some additional privileges for filtered syntax.
Ack


http://gerrit.cloudera.org:8080/#/c/23455/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23455/5//COMMIT_MSG@30
PS5, Line 30: synaxes
typo: syntaxes


http://gerrit.cloudera.org:8080/#/c/23455/3/fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
File fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java:

http://gerrit.cloudera.org:8080/#/c/23455/3/fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java@137
PS3, Line 137:     // To rewrite transforms and column references
             :     IcebergPartitionExpressionRewriter rewriter =
             :         new IcebergPartitionExpressionRewriter(analyzer,
             :             table.getIcebergApiTable().spec());
             :     // For Impala expression to Iceberg expression conversion
             :     IcebergPredicateConverter converter =
             :         new 
IcebergPartitionPredicateConverter(table.getIcebergSchema(), analyzer);
             :
             :     List<Expression> icebergPartitionExprs = new ArrayList<>();
             :     for (Expr expr : partitionSet_.getPartitionExprs()) {
             :       expr = rewriter.rewrite(expr);
             :       expr.analyze(analyzer);
             :       analyzer.getConstantFolder().rewrite(expr, analyzer);
             :       try {
             :         icebergPartitionExprs.add(converter.convert(expr));
             :       } catch (ImpalaException e) {
             :         throw new AnalysisException(
             :             "Invalid partition filtering expression: " + 
expr.toSql());
             :       }
             :     }
> I've tried to do it but it produced more java code than it is right now. I'
Ack


http://gerrit.cloudera.org:8080/#/c/23455/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:

http://gerrit.cloudera.org:8080/#/c/23455/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@4306
PS3, Line 4306: partition
> yeah, I'd leave it explicit IYDM though
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb9913e078e6842861bdbb004ed5d67286bd3152
Gerrit-Change-Number: 23455
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Mihaly Szjatinya <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Mon, 13 Oct 2025 13:37:17 +0000
Gerrit-HasComments: Yes

Reply via email to