[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[ https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16949061#comment-16949061 ] Wang Yanlin commented on CALCITE-3379: -- Agree that we need to validate both DDL and expression of table columns. > Support expand STRING column expression in table during sql-to-rel conversion > - > > Key: CALCITE-3379 > URL: https://issues.apache.org/jira/browse/CALCITE-3379 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.21.0 >Reporter: Danny Chen >Assignee: Danny Chen >Priority: Major > Labels: pull-request-available > Fix For: 1.22.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Now there are 2 ways to convert a RelOptTable to LogicalTableScan: > 1. One way is to open the Config#isConvertTableAccess[1] flag and the > SqlToRelConverter would invoke the #toRel method which transforms the table > to a node returned by the user(Usually a table scan). > 2. Another way is to use the LogicalTableScan rule, this rule would invoke > RelOptTable#toRel and wrap the returned node with a LogicalTableScan. > > The difference between 1 and 2 is that, 2 happens in the planning rule but 1 > happens in sql-to-rel conversion, 1 also supports to expand the table columns > based on the defined default values expressions, see > InitializerExpressionFactory#newColumnDefaultValue. > > The problem with the InitializerExpressionFactory#newColumnDefaultValue is > that it uses InitializerContext#convertExpression to convert a SqlNode, but > if the SqlNode is not validated, we always got a RexCall with > SqlUnresolvedFunction. We should give the user chance to validate their > SqlNode or even we can support pure string expressions which can be used to > persist. > > Another problem with #toRel is that after the expressions applied as a > projection, user has no chance to apply any other relational nodes if they > want, we can actually support this, the same way as we support the column > expressions. > > [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[ https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948301#comment-16948301 ] Danny Chen commented on CALCITE-3379: - Finally i decide not to support a pure string column expression, if user want to persist their expressions, they can firstly transform them to sql strings like "a + 1" or "my_udf(a)", then use the SqlParser to parse it back to SqlNode and transform it to RexNode with the InitializerContext. That means, use should control the expressions se-de by themselves. Personally i think the SqlToRelConverter should not see the parser thing anymore. So i would just close this issue. > Support expand STRING column expression in table during sql-to-rel conversion > - > > Key: CALCITE-3379 > URL: https://issues.apache.org/jira/browse/CALCITE-3379 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.21.0 >Reporter: Danny Chen >Assignee: Danny Chen >Priority: Major > Labels: pull-request-available > Fix For: 1.22.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Now there are 2 ways to convert a RelOptTable to LogicalTableScan: > 1. One way is to open the Config#isConvertTableAccess[1] flag and the > SqlToRelConverter would invoke the #toRel method which transforms the table > to a node returned by the user(Usually a table scan). > 2. Another way is to use the LogicalTableScan rule, this rule would invoke > RelOptTable#toRel and wrap the returned node with a LogicalTableScan. > > The difference between 1 and 2 is that, 2 happens in the planning rule but 1 > happens in sql-to-rel conversion, 1 also supports to expand the table columns > based on the defined default values expressions, see > InitializerExpressionFactory#newColumnDefaultValue. > > The problem with the InitializerExpressionFactory#newColumnDefaultValue is > that it uses InitializerContext#convertExpression to convert a SqlNode, but > if the SqlNode is not validated, we always got a RexCall with > SqlUnresolvedFunction. We should give the user chance to validate their > SqlNode or even we can support pure string expressions which can be used to > persist. > > Another problem with #toRel is that after the expressions applied as a > projection, user has no chance to apply any other relational nodes if they > want, we can actually support this, the same way as we support the column > expressions. > > [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[ https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948295#comment-16948295 ] Danny Chen commented on CALCITE-3379: - Okey, got your idea, personally, i'm inclined to validate the nodes when the query really happens, just like many other sql engines do to VIEWs [1]. The computed column actually is a VIEW. [1] https://docs.microsoft.com/en-us/sql/t-sql/statements/create-view-transact-sql?view=sql-server-2017#remarks > Support expand STRING column expression in table during sql-to-rel conversion > - > > Key: CALCITE-3379 > URL: https://issues.apache.org/jira/browse/CALCITE-3379 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.21.0 >Reporter: Danny Chen >Assignee: Danny Chen >Priority: Major > Labels: pull-request-available > Fix For: 1.22.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Now there are 2 ways to convert a RelOptTable to LogicalTableScan: > 1. One way is to open the Config#isConvertTableAccess[1] flag and the > SqlToRelConverter would invoke the #toRel method which transforms the table > to a node returned by the user(Usually a table scan). > 2. Another way is to use the LogicalTableScan rule, this rule would invoke > RelOptTable#toRel and wrap the returned node with a LogicalTableScan. > > The difference between 1 and 2 is that, 2 happens in the planning rule but 1 > happens in sql-to-rel conversion, 1 also supports to expand the table columns > based on the defined default values expressions, see > InitializerExpressionFactory#newColumnDefaultValue. > > The problem with the InitializerExpressionFactory#newColumnDefaultValue is > that it uses InitializerContext#convertExpression to convert a SqlNode, but > if the SqlNode is not validated, we always got a RexCall with > SqlUnresolvedFunction. We should give the user chance to validate their > SqlNode or even we can support pure string expressions which can be used to > persist. > > Another problem with #toRel is that after the expressions applied as a > projection, user has no chance to apply any other relational nodes if they > want, we can actually support this, the same way as we support the column > expressions. > > [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[
https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948284#comment-16948284
]
Wang Yanlin commented on CALCITE-3379:
--
it's *trims* not *trim*.
Trim is valid, but trims is invalid.
Or, just check using a *invalid_function* call like this
{noformat}
final String create = "create table t1 (\n"
+ " h varchar(3) not null,\n"
+ " i varchar(3),\n"
+ " j int not null as (char_length(h)) virtual,\n"
+ " k varchar(3) null as (invalid_function(i)) virtual)";
boolean b = s.execute(create);
assertThat(b, is(false));
int x = s.executeUpdate("insert into t1 (h, i) values ('abc', 'de ')");
assertThat(x, is(1));
{noformat}
> Support expand STRING column expression in table during sql-to-rel conversion
> -
>
> Key: CALCITE-3379
> URL: https://issues.apache.org/jira/browse/CALCITE-3379
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.21.0
>Reporter: Danny Chen
>Assignee: Danny Chen
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> Now there are 2 ways to convert a RelOptTable to LogicalTableScan:
> 1. One way is to open the Config#isConvertTableAccess[1] flag and the
> SqlToRelConverter would invoke the #toRel method which transforms the table
> to a node returned by the user(Usually a table scan).
> 2. Another way is to use the LogicalTableScan rule, this rule would invoke
> RelOptTable#toRel and wrap the returned node with a LogicalTableScan.
>
> The difference between 1 and 2 is that, 2 happens in the planning rule but 1
> happens in sql-to-rel conversion, 1 also supports to expand the table columns
> based on the defined default values expressions, see
> InitializerExpressionFactory#newColumnDefaultValue.
>
> The problem with the InitializerExpressionFactory#newColumnDefaultValue is
> that it uses InitializerContext#convertExpression to convert a SqlNode, but
> if the SqlNode is not validated, we always got a RexCall with
> SqlUnresolvedFunction. We should give the user chance to validate their
> SqlNode or even we can support pure string expressions which can be used to
> persist.
>
> Another problem with #toRel is that after the expressions applied as a
> projection, user has no chance to apply any other relational nodes if they
> want, we can actually support this, the same way as we support the column
> expressions.
>
> [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[ https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948269#comment-16948269 ] Danny Chen commented on CALCITE-3379: - > And exception occurs when trying to query the table. [~yanlin-Lynn] What do you mean by an exception throws ? The query in the test case is valid. > Support expand STRING column expression in table during sql-to-rel conversion > - > > Key: CALCITE-3379 > URL: https://issues.apache.org/jira/browse/CALCITE-3379 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.21.0 >Reporter: Danny Chen >Assignee: Danny Chen >Priority: Major > Labels: pull-request-available > Fix For: 1.22.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Now there are 2 ways to convert a RelOptTable to LogicalTableScan: > 1. One way is to open the Config#isConvertTableAccess[1] flag and the > SqlToRelConverter would invoke the #toRel method which transforms the table > to a node returned by the user(Usually a table scan). > 2. Another way is to use the LogicalTableScan rule, this rule would invoke > RelOptTable#toRel and wrap the returned node with a LogicalTableScan. > > The difference between 1 and 2 is that, 2 happens in the planning rule but 1 > happens in sql-to-rel conversion, 1 also supports to expand the table columns > based on the defined default values expressions, see > InitializerExpressionFactory#newColumnDefaultValue. > > The problem with the InitializerExpressionFactory#newColumnDefaultValue is > that it uses InitializerContext#convertExpression to convert a SqlNode, but > if the SqlNode is not validated, we always got a RexCall with > SqlUnresolvedFunction. We should give the user chance to validate their > SqlNode or even we can support pure string expressions which can be used to > persist. > > Another problem with #toRel is that after the expressions applied as a > projection, user has no chance to apply any other relational nodes if they > want, we can actually support this, the same way as we support the column > expressions. > > [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[
https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948237#comment-16948237
]
Wang Yanlin commented on CALCITE-3379:
--
Currently, this test case in *ServerTest* run success, mind the invalid use of
*trims*.
{noformat}
@Test public void testVirtualColumnWithFunctions() throws Exception {
try (Connection c = connect();
Statement s = c.createStatement()) {
// Test builtin and library functions.
final String create = "create table t1 (\n"
+ " h varchar(3) not null,\n"
+ " i varchar(3),\n"
+ " j int not null as (char_length(h)) virtual,\n"
+ " k varchar(3) null as (trims(i)) virtual)";
boolean b = s.execute(create);
assertThat(b, is(false));
int x = s.executeUpdate("insert into t1 (h, i) values ('abc', 'de ')");
assertThat(x, is(1));
}
}
{noformat}
And exception occurs when trying to query the table.
Agree with [~jinxing6...@126.com], it would be better to throw an exception
when creating the table.
> Support expand STRING column expression in table during sql-to-rel conversion
> -
>
> Key: CALCITE-3379
> URL: https://issues.apache.org/jira/browse/CALCITE-3379
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.21.0
>Reporter: Danny Chen
>Assignee: Danny Chen
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> Now there are 2 ways to convert a RelOptTable to LogicalTableScan:
> 1. One way is to open the Config#isConvertTableAccess[1] flag and the
> SqlToRelConverter would invoke the #toRel method which transforms the table
> to a node returned by the user(Usually a table scan).
> 2. Another way is to use the LogicalTableScan rule, this rule would invoke
> RelOptTable#toRel and wrap the returned node with a LogicalTableScan.
>
> The difference between 1 and 2 is that, 2 happens in the planning rule but 1
> happens in sql-to-rel conversion, 1 also supports to expand the table columns
> based on the defined default values expressions, see
> InitializerExpressionFactory#newColumnDefaultValue.
>
> The problem with the InitializerExpressionFactory#newColumnDefaultValue is
> that it uses InitializerContext#convertExpression to convert a SqlNode, but
> if the SqlNode is not validated, we always got a RexCall with
> SqlUnresolvedFunction. We should give the user chance to validate their
> SqlNode or even we can support pure string expressions which can be used to
> persist.
>
> Another problem with #toRel is that after the expressions applied as a
> projection, user has no chance to apply any other relational nodes if they
> want, we can actually support this, the same way as we support the column
> expressions.
>
> [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[ https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948187#comment-16948187 ] Danny Chen commented on CALCITE-3379: - Yes, column expression need to be validated, this is resolved in CALCITE-3392. > Support expand STRING column expression in table during sql-to-rel conversion > - > > Key: CALCITE-3379 > URL: https://issues.apache.org/jira/browse/CALCITE-3379 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.21.0 >Reporter: Danny Chen >Assignee: Danny Chen >Priority: Major > Labels: pull-request-available > Fix For: 1.22.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Now there are 2 ways to convert a RelOptTable to LogicalTableScan: > 1. One way is to open the Config#isConvertTableAccess[1] flag and the > SqlToRelConverter would invoke the #toRel method which transforms the table > to a node returned by the user(Usually a table scan). > 2. Another way is to use the LogicalTableScan rule, this rule would invoke > RelOptTable#toRel and wrap the returned node with a LogicalTableScan. > > The difference between 1 and 2 is that, 2 happens in the planning rule but 1 > happens in sql-to-rel conversion, 1 also supports to expand the table columns > based on the defined default values expressions, see > InitializerExpressionFactory#newColumnDefaultValue. > > The problem with the InitializerExpressionFactory#newColumnDefaultValue is > that it uses InitializerContext#convertExpression to convert a SqlNode, but > if the SqlNode is not validated, we always got a RexCall with > SqlUnresolvedFunction. We should give the user chance to validate their > SqlNode or even we can support pure string expressions which can be used to > persist. > > Another problem with #toRel is that after the expressions applied as a > projection, user has no chance to apply any other relational nodes if they > want, we can actually support this, the same way as we support the column > expressions. > > [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[
https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16946519#comment-16946519
]
jin xing commented on CALCITE-3379:
---
As Danny explained by the first point, I have one concern – Do we really don't
consider to validate SqlNode for DDL.
For example, when user create a table by below sql with a typo
{code:java}
create table if not exists x (
i int not null,
m int as (sqrtt(q + 1)) virtual)
{code}
I think it would benefit to throws exception and tell "sqrtt" cannot be
resolved.
Say data provider and consumer are from different teams. It would be better to
catch the mistake when providing the data, rather than the time using data.
> Support expand STRING column expression in table during sql-to-rel conversion
> -
>
> Key: CALCITE-3379
> URL: https://issues.apache.org/jira/browse/CALCITE-3379
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.21.0
>Reporter: Danny Chen
>Assignee: Danny Chen
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> Now there are 2 ways to convert a RelOptTable to LogicalTableScan:
> 1. One way is to open the Config#isConvertTableAccess[1] flag and the
> SqlToRelConverter would invoke the #toRel method which transforms the table
> to a node returned by the user(Usually a table scan).
> 2. Another way is to use the LogicalTableScan rule, this rule would invoke
> RelOptTable#toRel and wrap the returned node with a LogicalTableScan.
>
> The difference between 1 and 2 is that, 2 happens in the planning rule but 1
> happens in sql-to-rel conversion, 1 also supports to expand the table columns
> based on the defined default values expressions, see
> InitializerExpressionFactory#newColumnDefaultValue.
>
> The problem with the InitializerExpressionFactory#newColumnDefaultValue is
> that it uses InitializerContext#convertExpression to convert a SqlNode, but
> if the SqlNode is not validated, we always got a RexCall with
> SqlUnresolvedFunction. We should give the user chance to validate their
> SqlNode or even we can support pure string expressions which can be used to
> persist.
>
> Another problem with #toRel is that after the expressions applied as a
> projection, user has no chance to apply any other relational nodes if they
> want, we can actually support this, the same way as we support the column
> expressions.
>
> [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3379) Support expand STRING column expression in table during sql-to-rel conversion
[ https://issues.apache.org/jira/browse/CALCITE-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16946450#comment-16946450 ] Danny Chen commented on CALCITE-3379: - [~julianhyde] [~hyuan],yes, the linked PR is not relevant with this JIRA, maybe i mis-clicked. Let me re-state the use case: Now user can declare the column of a table in DDL with special strategy(known as column constraints in other sql engines[1] [2]), as we enumerate in ColumnStrategy[3], then with the SqlNode got from InitializerExpressionFactory#newColumnDefaultValue, SqlToRelConverter would translate it to a RexCall and put it the project list above the table. InitializerExpressionFactory#newColumnDefaultValue use the InitializerContext to convert the SqlNode, the problem is that: # SqlNode got from the DDL is not validated(we have no validation for DDL nodes yet, and i think we don'tneed to) and we always got a RexCall with SqlUnresolvedFunction. # The SqlNode can not be serialized directly, many times the DDL definition need to persist into the Catalog as a metadata(we may unparse it to pure sql but that is not intuitive and we still need use to parse it back to SqlNode when deserializing) So, what i proposed is that add the InitializerContext#convertExpression(String) and deprecate the #convertExpression(SqlNode), Calcite is the role to parse the string to SqlNode then validate and translate it to RexCall. What the user see is always a string and it can be persisted. > Support expand STRING column expression in table during sql-to-rel conversion > - > > Key: CALCITE-3379 > URL: https://issues.apache.org/jira/browse/CALCITE-3379 > Project: Calcite > Issue Type: Improvement > Components: core >Affects Versions: 1.21.0 >Reporter: Danny Chen >Assignee: Danny Chen >Priority: Major > Labels: pull-request-available > Fix For: 1.22.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Now there are 2 ways to convert a RelOptTable to LogicalTableScan: > 1. One way is to open the Config#isConvertTableAccess[1] flag and the > SqlToRelConverter would invoke the #toRel method which transforms the table > to a node returned by the user(Usually a table scan). > 2. Another way is to use the LogicalTableScan rule, this rule would invoke > RelOptTable#toRel and wrap the returned node with a LogicalTableScan. > > The difference between 1 and 2 is that, 2 happens in the planning rule but 1 > happens in sql-to-rel conversion, 1 also supports to expand the table columns > based on the defined default values expressions, see > InitializerExpressionFactory#newColumnDefaultValue. > > The problem with the InitializerExpressionFactory#newColumnDefaultValue is > that it uses InitializerContext#convertExpression to convert a SqlNode, but > if the SqlNode is not validated, we always got a RexCall with > SqlUnresolvedFunction. We should give the user chance to validate their > SqlNode or even we can support pure string expressions which can be used to > persist. > > Another problem with #toRel is that after the expressions applied as a > projection, user has no chance to apply any other relational nodes if they > want, we can actually support this, the same way as we support the column > expressions. > > [1]https://github.com/apache/calcite/blob/2dc97e6723e1b5bf762540f87b5cd1a848a1/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5605 -- This message was sent by Atlassian Jira (v8.3.4#803005)
