Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23536 )

Change subject: IMPALA-13066: Extend SHOW CREATE TABLE to include stats and 
partitions
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/23536/8/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/23536/8/be/src/service/client-request-state.cc@575
PS8, Line 575: 1000; // Default value
Can init with this?

  query_options().partition_limit;


http://gerrit.cloudera.org:8080/#/c/23536/8/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/23536/8/common/thrift/Query.thrift@813
PS8, Line 813: ;
Please add test with PARTITION_LIMIT=1 over table with more than 1 partition, 
and validate accordingly.

nit: semicolon is actually not needed.


http://gerrit.cloudera.org:8080/#/c/23536/8/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:

http://gerrit.cloudera.org:8080/#/c/23536/8/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@243
PS8, Line 243:   private static CreateSqlArtifacts renderCreateTableSql(FeTable 
table)
             :       throws CatalogException {
Please move this method just below getCreateTableSql() so the diff to review is 
smaller.


http://gerrit.cloudera.org:8080/#/c/23536/8/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@845
PS8, Line 845:           // Add warning if partitions were skipped
             :           if (partitionLimit > 0 && totalPartitions > 
partitionLimit) {
             :             int skipped = totalPartitions - partitionLimit;
             :             warnings.append("-- WARNING about partial output\n");
             :             warnings.append("-- WARNING: Emitted 
").append(partitionLimit)
             :                 .append(" of ").append(totalPartitions)
             :                 .append(" partitions 
(partition_limit=").append(partitionLimit)
             :                 .append("). ").append(skipped).append(" 
partitions skipped.\n")
             :                 .append("-- To export more partitions, re-run 
with PARTITION_LIMIT=<n>.\n");
             :           }
Can this WARNING be validated in the test file?


http://gerrit.cloudera.org:8080/#/c/23536/8/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/23536/8/fe/src/main/java/org/apache/impala/service/Frontend.java@685
PS8, Line 685: 1000
Can be replaced with default value from TQueryOption.

  (new TQueryOptions()).getPartition_limit();



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I87950ae9d9bb73cb2a435cf5bcad076df1570dc2
Gerrit-Change-Number: 23536
Gerrit-PatchSet: 8
Gerrit-Owner: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Thu, 23 Oct 2025 04:29:07 +0000
Gerrit-HasComments: Yes

Reply via email to