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