twalthr commented on code in PR #24993:
URL: https://github.com/apache/flink/pull/24993#discussion_r1672253123


##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlCreateTableAs.java:
##########
@@ -124,18 +124,6 @@ public void validate() throws SqlValidateException {
                     getParserPosition(),
                     "CREATE TABLE AS SELECT syntax does not support to create 
temporary table yet.");
         }
-
-        if (getDistribution() != null) {
-            throw new SqlValidateException(
-                    getParserPosition(),
-                    "CREATE TABLE AS SELECT syntax does not support creating 
distributed tables yet.");
-        }
-        // TODO flink dialect supports dynamic partition

Review Comment:
   dynamic partitions are like:
   ```
   INSERT INTO t PARTITION (region='europe') SELECT a, b, c, month FROM 
another_view
   ```
   
   can you quickly check if the parser supports this? I guess no because it is 
part of the INSERT INTO?



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlCreateTableAs.java:
##########
@@ -124,18 +124,6 @@ public void validate() throws SqlValidateException {
                     getParserPosition(),
                     "CREATE TABLE AS SELECT syntax does not support to create 
temporary table yet.");
         }
-
-        if (getDistribution() != null) {
-            throw new SqlValidateException(
-                    getParserPosition(),
-                    "CREATE TABLE AS SELECT syntax does not support creating 
distributed tables yet.");
-        }
-        // TODO flink dialect supports dynamic partition

Review Comment:
   my intention is to ensure we are not silently dropping them. we don't need 
to support them for CREATE TABLE AS



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlDdlToOperationConverterTest.java:
##########
@@ -856,18 +858,30 @@ public void testMergingCreateTableAsWitDistribution() {
                                         .build())
                         
.distribution(TableDistribution.ofHash(Collections.singletonList("f0"), 3))
                         .partitionKeys(Arrays.asList("f0", "f1"))
-                        .options(sourceProperties)

Review Comment:
   remove unused `sourceProperties` variable



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlDdlToOperationConverterTest.java:
##########
@@ -856,18 +858,30 @@ public void testMergingCreateTableAsWitDistribution() {
                                         .build())
                         
.distribution(TableDistribution.ofHash(Collections.singletonList("f0"), 3))
                         .partitionKeys(Arrays.asList("f0", "f1"))
-                        .options(sourceProperties)

Review Comment:
   fix typo in name of the method



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to