wuchong commented on code in PR #22208:
URL: https://github.com/apache/flink/pull/22208#discussion_r1141276168
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConverters.java:
##########
@@ -37,9 +40,9 @@ public class SqlNodeConverters {
static {
// register all the converters here
register(new SqlCreateCatalogConverter());
- register(new SqlCreateTableAsConverter());
- register(new SqlCreateTableLikeConverter());
- register(new SqlCreateTableConverter());
+ register(new SqlCreateTableConverter(),
TypeInformation.of(SqlCreateTable.class));
+ register(new SqlCreateTableConverter(),
TypeInformation.of(SqlCreateTableAs.class));
+ register(new SqlCreateTableConverter(),
TypeInformation.of(SqlCreateTableLike.class));
Review Comment:
IMO, this breaks the design of `SqlNodeConverter` somehow. The principle of
`SqlNodeConverter` focuses on a single kind of SqlNode conversion, but this
involves `if else` and `instance of` which we would like to avoid in
`SqlNodeToOperationConversion`.
Is there any problems to continue the way of 3 separate converters?
--
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
