korlov42 commented on a change in pull request #9119:
URL: https://github.com/apache/ignite/pull/9119#discussion_r713153348
##########
File path: modules/calcite/src/main/codegen/includes/parserImpls.ftl
##########
@@ -148,19 +148,29 @@ SqlCreate SqlCreateTable(Span s, boolean replace) :
final SqlIdentifier id;
final SqlNodeList columnList;
final SqlNodeList optionList;
+ final SqlNode query;
}
{
<TABLE>
ifNotExists = IfNotExistsOpt()
id = CompoundIdentifier()
- columnList = TableElementList()
+ (
+ columnList = TableElementList()
Review comment:
AFAIK column list in case of `CREATE TABLE ... AS` should contain only
aliases without any types specs or constraints
##########
File path: modules/calcite/src/main/codegen/includes/parserImpls.ftl
##########
@@ -148,19 +148,29 @@ SqlCreate SqlCreateTable(Span s, boolean replace) :
final SqlIdentifier id;
final SqlNodeList columnList;
final SqlNodeList optionList;
+ final SqlNode query;
}
{
<TABLE>
ifNotExists = IfNotExistsOpt()
id = CompoundIdentifier()
- columnList = TableElementList()
+ (
+ columnList = TableElementList()
+ |
+ { columnList = null; }
+ )
+ (
+ <AS> query = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY)
Review comment:
it's better to have `AS` part as a last one to not to mess to which part
of a statement a `WITH` clause belongs to. For example:
```
1) CREATE TABLE my_t AS SELECT a, b FROM another_T WITH backups=3
2) CREATE TABLE my_t WITH backups=3 AS SELECT a, b FROM another_T
```
The second option seems better to me.
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/PlannerHelper.java
##########
@@ -107,6 +121,72 @@ public static IgniteRel optimize(SqlNode sqlNode,
IgnitePlanner planner, IgniteL
}
}
+ /**
+ * Creates physical plan for "INSERT INTO table SELECT ..." based on
"CREATE TABLE table AS SELECT ..." statement.
+ */
+ public static IgniteRel makeCreateTableAsSelectPlan(CreateTableCommand
createTableCmd, PlanningContext ctx,
Review comment:
Honestly speaking, I don't like the idea to create execution plan by
hand. First, the table internals has leaked to the the planner level (row type
format to feed to ModifyNode). And second, in such case we need to reimplement
here all optimizations which planner would possibly apply. For example like
this one https://issues.apache.org/jira/browse/IGNITE-12692.
Have you considered an implicit rewriting of query with AS clause to two
distinct statements (`CREATE` + `INSERT`)?
##########
File path: modules/calcite/src/main/codegen/includes/parserImpls.ftl
##########
@@ -148,19 +148,29 @@ SqlCreate SqlCreateTable(Span s, boolean replace) :
final SqlIdentifier id;
final SqlNodeList columnList;
final SqlNodeList optionList;
+ final SqlNode query;
}
{
<TABLE>
ifNotExists = IfNotExistsOpt()
id = CompoundIdentifier()
- columnList = TableElementList()
+ (
+ columnList = TableElementList()
+ |
+ { columnList = null; }
+ )
+ (
+ <AS> query = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY)
+ |
+ { query = null; }
+ )
(
<WITH> { s.add(this); } optionList = CreateTableOptionList()
|
{ optionList = null; }
)
{
Review comment:
Please extend
`org.apache.ignite.internal.processors.query.calcite.sql.SqlDdlParserTest` with
tests to verify the new clause format
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
