Arnab Karmakar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23566 )

Change subject: IMPALA-14065: Support WHERE clause in SHOW PARTITIONS statement
......................................................................


Patch Set 11:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/23566/10/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/23566/10/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@206
PS10, Line 206:       List<CompoundVerticalBarExpr> compoundVerticalBarExprs = 
new ArrayList<>();
              :       
whereClause_.collectAll(Predicates.instanceOf(CompoundVerticalBarExpr.class),
              :           compoundVerticalBarExprs);
              :       if (!compoundVerticalBarExprs.isEmpty()) {
              :         // Expression needs rewriting - defer partition 
filtering to second analysis.
              :         return;
              :       }
> Please add tests for such expressions.
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@224
PS10, Line 224:         throw new AnalysisException(
              :           "Aggregate functions are not supported in SHOW 
PARTITIONS WHERE clause");
              :       }
              :       // Ensure all conjuncts reference only partition columns.
              :       Lis
> Can we simply check whereClause_.isBoundBySlotIds(partitionSlots) ?
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@230
PS10, Line 230:         throw new AnalysisException(
              :           "SHOW PARTITIONS WHERE supports only partition 
columns");
              :       }
              :
> Can we simply check whereClause_.contains(Expr.IS_AGGREGATE) like above for
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@244
PS10, Line 244:         Preconditions.checkState(conjunctsCopy.isEmpty(),
> Please add a Precondition check on conjunctsCopy.isEmpty() after this, whic
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
File fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java:

http://gerrit.cloudera.org:8080/#/c/23566/10/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java@113
PS10, Line 113:    * If 'evalAllFuncs' is True, non-deterministic functions 
will be evaluated
              :    * per-partition instead of being skipped.
> nit: please also mention that callers should make sure the conjects are all
Done, though it wont exactly crash but we'll get an Analysis Exception


http://gerrit.cloudera.org:8080/#/c/23566/10/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java@118
PS10, Line 118: nds FeFsPartition>, Li
> nit: though we prefer adding new parameters at the end, it looks better to
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/testdata/workloads/functional-query/queries/QueryTest/show-stats.test
File testdata/workloads/functional-query/queries/QueryTest/show-stats.test:

http://gerrit.cloudera.org:8080/#/c/23566/10/testdata/workloads/functional-query/queries/QueryTest/show-stats.test@312
PS10, Line 312: month = 1 or month = 6
> nit: use 'month' to reduce the result size instead of returning all partiti
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/testdata/workloads/functional-query/queries/QueryTest/show-stats.test@447
PS10, Line 447: 'Total','',620,2,'40.73KB','0B','','','','',''
              : ---- TYPES
> This is an implementation-specific detail and should not be relied upon. I
Done


http://gerrit.cloudera.org:8080/#/c/23566/10/testdata/workloads/functional-query/queries/QueryTest/show-stats.test@495
PS10, Line 495: ---- CATCH
> It would be better to test this in python code where we can verify the resu
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e2a14aabcea3fb17083d4ad6f87b7861113f89e
Gerrit-Change-Number: 23566
Gerrit-PatchSet: 11
Gerrit-Owner: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Surya Hebbar <[email protected]>
Gerrit-Comment-Date: Wed, 03 Dec 2025 10:16:04 +0000
Gerrit-HasComments: Yes

Reply via email to