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

Change subject: IMPALA-13299: Support CREATE TABLE LIKE for Iceberg from 
non-Iceberg sources
......................................................................


Patch Set 3:

(5 comments)

Thanks for working on this!

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

http://gerrit.cloudera.org:8080/#/c/23733/3//COMMIT_MSG@7
PS3, Line 7: non-Iceberg sources
Title could be more precise, as e.g. Kudu tables are not supported as sources.

And in all the tests I only see plain Parquet tables as sources.


http://gerrit.cloudera.org:8080/#/c/23733/3//COMMIT_MSG@13
PS3, Line 13: which is particularly important for
            : Apache Hive 3.1 compatibility where CTAS with STORED BY ICEBERG 
is not
            : supported.
What do you mean by this sentence?

CTAS with STORED BY ICEBERG should work, no matter what is the source table 
type. As long as the data types are supported by Iceberg.


http://gerrit.cloudera.org:8080/#/c/23733/3/docs/topics/impala_iceberg.xml
File docs/topics/impala_iceberg.xml:

http://gerrit.cloudera.org:8080/#/c/23733/3/docs/topics/impala_iceberg.xml@1030
PS3, Line 1030: </p>
You could add a section about limitations:

 * unsupported source table types
 * unsupported data types


http://gerrit.cloudera.org:8080/#/c/23733/3/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/23733/3/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@178
PS3, Line 178:     // Note: Creating non-Iceberg tables from Iceberg sources 
works
             :     // Validate that JDBC and Paimon tables cannot be used as 
source
Not sure about the first sentence: "Creating non-Iceberg tables from Iceberg 
sources works". Do you test this? I think we have a bug here as the following 
created an Iceberg table for me despite the 'stored as parquet' clause:
create table like_t like ice_source stored as parquet;

Second sentence is not valid about JDBC tables, as we raise an exception if the 
target table is a JDBC table, not when the source.

Paimon tables cannot be used as source or target.


http://gerrit.cloudera.org:8080/#/c/23733/3/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/23733/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4673
PS3, Line 4673:       // Add non-partitioning columns first
              :       for (Column col: srcTable.getNonClusteringColumns()) {
              :         TColumn tcol = col.toThrift();
              :         // Default to nullable for Iceberg (optional fields) if 
not explicitly set
              :         if (!tcol.isSetIs_nullable()) {
              :           tcol.setIs_nullable(true);
              :         }
              :         columns.add(tcol);
              :       }
              :
              :       // Then add partitioning columns
              :       for (Column col: srcTable.getClusteringColumns()) {
              :         TColumn tcol = col.toThrift();
              :         // Default to nullable for Iceberg (optional fields) if 
not explicitly set
              :         if (!tcol.isSetIs_nullable()) {
              :           tcol.setIs_nullable(true);
              :         }
              :         columns.add(tcol);
              :       }
Can you use a single for-loop with getColumnsInHiveOrder()?

      for (Column col: srcTable.getColumnsInHiveOrder()) {
        TColumn tcol = col.toThrift();
        // Default to nullable for Iceberg (optional fields) if not explicitly 
set
        if (!tcol.isSetIs_nullable()) {
          tcol.setIs_nullable(true);
        }
        columns.add(tcol);
      }



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id162f217e49e9f396419b09815b92eb7f351881e
Gerrit-Change-Number: 23733
Gerrit-PatchSet: 3
Gerrit-Owner: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Vanko <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Mihaly Szjatinya <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 28 Jan 2026 13:48:22 +0000
Gerrit-HasComments: Yes

Reply via email to