Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24609 )

Change subject: IMPALA-10693: Allow TBLPROPERTIES in CREATE TABLE LIKE
......................................................................


Patch Set 5:

(15 comments)

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

http://gerrit.cloudera.org:8080/#/c/24609/5//COMMIT_MSG@26
PS5, Line 26: handling Iceberg properties appropriately
Please add e2e tests for Iceberg tables:

Iceberg -> Iceberg
Non-Iceberg -> Iceberg
Iceberg -> Non-Iceberg


http://gerrit.cloudera.org:8080/#/c/24609/5//COMMIT_MSG@30
PS5, Line 30: property validation
Properties are not validates. We'd need e2e tests for this with SHOW CREATE 
TABLE/DESCRIBE statements.


http://gerrit.cloudera.org:8080/#/c/24609/5//COMMIT_MSG@74
PS5, Line 74: 2.
Not corresponding '1.'


http://gerrit.cloudera.org:8080/#/c/24609/5//COMMIT_MSG@74
PS5, Line 74: This change allows users to override or add table properties
            :    during a CREATE TABLE LIKE statement.
            :
            : For example, specifying 'transactional'='true' now executes
            : successfully instead of throwing a grammar ParseException.
            :
            : CREATE TABLE orders_acid LIKE orders
            : STORED AS ORC
            : TBLPROPERTIES ('transactional'='true')
            :
            : ParseException: Syntax error in line 3:undefined: TBLPROPERTIES
            :  ('transactional'='true') ^ Encountered: TBLPROPERTIES Expected:
            :  LOCATION CAUSED BY: Exception: Syntax error
            :
            : After this change:
            :
            : [localhost:21050] default> show create table source_tbl;
            : Query: show create table source_tbl
            : +-------------------------------------------------------------+
            : | result                                                      |
            : +-------------------------------------------------------------+
            : | CREATE EXTERNAL TABLE default.source_tbl (                  |
            : |   id INT                                                    |
            : | )                                                           |
            : | STORED AS TEXTFILE                                          |
            : | LOCATION 'hdfs://localhost:20500/test-warehouse/source_tbl' |
            : | TBLPROPERTIES (                                             |
            : |   'OBJCAPABILITIES'='EXTREAD,EXTWRITE',                     |
            : |   'TRANSLATED_TO_EXTERNAL'='TRUE',                          |
            : |   'env'='prod',                                             |
            : |   'external.table.purge'='TRUE',                            |
            : |   'owner'='alice'                                           |
            : | )                                                           |
            : +-------------------------------------------------------------+
            :
            : [localhost:21050] default> CREATE TABLE target_tbl LIKE source_tbl
            :                          > 
TBLPROPERTIES('env'='dev','team'='analytics');
            : Query: CREATE TABLE target_tbl LIKE source_tbl
            : TBLPROPERTIES('env'='dev','team'='analytics')
            : +-------------------------+
            : | summary                 |
            : +-------------------------+
            : | Table has been created. |
            : +-------------------------+
            : Fetched 1 row(s) in 0.10s
            : [localhost:21050] default> show create table target_tbl;
            : Query: show create table target_tbl
            : +-------------------------------------------------------------+
            : | result                                                      |
            : +-------------------------------------------------------------+
            : | CREATE EXTERNAL TABLE default.target_tbl (                  |
            : |   id INT                                                    |
            : | )                                                           |
            : | STORED AS TEXTFILE                                          |
            : | LOCATION 'hdfs://localhost:20500/test-warehouse/target_tbl' |
            : | TBLPROPERTIES (                                             |
            : |   'OBJCAPABILITIES'='EXTREAD,EXTWRITE',                     |
            : |   'TRANSLATED_TO_EXTERNAL'='TRUE',                          |
            : |   'env'='dev',                                              |
            : |   'external.table.purge'='TRUE',                            |
            : |   'owner'='alice',                                          |
            : |   'team'='analytics'                                        |
            : | )                                                           |
            : +-------------------------------------------------------------+
Most of it was already explained above.


http://gerrit.cloudera.org:8080/#/c/24609/5//COMMIT_MSG@139
PS5, Line 139: Tests Performed:
Please add new end-to-end tests as well.


http://gerrit.cloudera.org:8080/#/c/24609/5/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/24609/5/common/thrift/JniCatalog.thrift@586
PS5, Line 586: NEW:
no need to say it's "NEW"


http://gerrit.cloudera.org:8080/#/c/24609/5/common/thrift/JniCatalog.thrift@586
PS5, Line 586:
indendation is off


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/cup/sql-parser.cup@1762
PS5, Line 1762:
nit: space at end of line


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/cup/sql-parser.cup@1764
PS5, Line 1764: opt_tbl_properties
We should just use the rule 'tbl_properties'. Empty properties map should be a 
no-op for CreateTableLikeStmt.


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/cup/sql-parser.cup@1776
PS5, Line 1776:
nit: space at end of line


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/cup/sql-parser.cup@2435
PS5, Line 2435:
extra white spaces


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java:

http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@76
PS5, Line 76:    * @param ifNotExists - If true, no errors are thrown if the 
table already exists
Please update JavaDoc comment with the new param.


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@93
PS5, Line 93: tblProperties_
Validate properties map in Analyze by using 
MetaStoreUtil.checkShortPropertyMap().


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@147
PS5, Line 147:     sb.append(" TBLPROPERTIES (");
Missing indentation


http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/24609/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4614
PS5, Line 4614:     if (params.isSetTbl_properties()) {
              :       tbl.getParameters().putAll(params.getTbl_properties());
              :     }
Why is it invoked twice?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d45bdfeebbc8107abd51f87145330dbbb675855
Gerrit-Change-Number: 24609
Gerrit-PatchSet: 5
Gerrit-Owner: Gowthami Bisati <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 04 Aug 2026 13:20:46 +0000
Gerrit-HasComments: Yes

Reply via email to