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 9:

(7 comments)

Thanks for continue working on this. I just have few remaining comments.

http://gerrit.cloudera.org:8080/#/c/23536/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23536/9//COMMIT_MSG@8
PS9, Line 8:
           : Add a WITH STATS option to SHOW CREATE TABLE that emits additional 
statements to recreate table statistics and partitions.
           :
           : When WITH STATS is specified, Impala emits:
           :
           : - Base CREATE TABLE statement.
           : - Table-level ALTER TABLE ... SET TBLPROPERTIES to set properties 
such as numRows and STATS_GENERATED_VIA_STATS_TASK.
           : - ALTER TABLE ... SET COLUMN STATS for all non-partition columns 
(column stats such as numDVs, numNulls, maxSize, avgSize, numTrues, and 
numFalses).
           : - For partitioned tables:
           :   - ALTER TABLE ... ADD PARTITION statements to recreate 
partitions.
           :   - Per-partition ALTER TABLE ... PARTITION (...) SET 
TBLPROPERTIES to restore partition-level stats like numRows, numFiles, and 
totalSize.
           :
           : Partition output is limited by the PARTITION_LIMIT query option 
(default 1000). Set PARTITION_LIMIT=0 to include all partitions; a warning is 
emitted if the limit is exceeded.
           : Parser, analyzer, and frontend plan are updated to support the new 
option. Tests added to verify table-, column-, and partition-level statements. 
Default SHOW CREATE TABLE behavior remains unchanged for backward compatibility.
nit: Please make it so that each commit message lines are 72 chars at most.

See "Fix" section at:
https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala


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: query_options().partit
> Can init with this?
Done


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 partitio
Done


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:    * ordering), if 'properties' doesn't contain 'sort.order'.
             :    */
> Please move this method just below getCreateTableSql() so the diff to revie
Done


http://gerrit.cloudera.org:8080/#/c/23536/8/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@845
PS8, Line 845:           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?
Done


http://gerrit.cloudera.org:8080/#/c/23536/9/testdata/workloads/functional-query/queries/QueryTest/show-create-table-with-stats.test
File 
testdata/workloads/functional-query/queries/QueryTest/show-create-table-with-stats.test:

http://gerrit.cloudera.org:8080/#/c/23536/9/testdata/workloads/functional-query/queries/QueryTest/show-create-table-with-stats.test@340
PS9, Line 340: ====
             : ---- CREATE_TABLE
Please add one test case that test against existing table.

====
---- QUERY
SHOW CREATE TABLE functional.alltypes_date_partition WITH ARGS


http://gerrit.cloudera.org:8080/#/c/23536/9/tests/metadata/test_show_create_table.py
File tests/metadata/test_show_create_table.py:

http://gerrit.cloudera.org:8080/#/c/23536/9/tests/metadata/test_show_create_table.py@186
PS9, Line 186: if not is_paimon:
No need to COMPUTE STATS on existing table.

  if not is_paimon and not test_case.existing_table:



--
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: 9
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 23:23:26 +0000
Gerrit-HasComments: Yes

Reply via email to