[jira] [Resolved] (CALCITE-3973) Hints should not unparse as enclosed in parentheses

2020-05-06 Thread Danny Chen (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Danny Chen resolved CALCITE-3973.
-
Resolution: Fixed

Fixed in 
[22fca8e|https://github.com/apache/calcite/commit/22fca8e43d62ffae12b7d7e1a50438f257ac88b3],
 thanks for your PR [~alexbaden] !

> Hints should not unparse as enclosed in parentheses
> ---
>
> Key: CALCITE-3973
> URL: https://issues.apache.org/jira/browse/CALCITE-3973
> Project: Calcite
>  Issue Type: Bug
>Affects Versions: 1.22.0
>Reporter: Alex Baden
>Priority: Minor
> Fix For: 1.23.0
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When using the new SQL hints feature:
>  
> {code:java}
> select /*+ cpu */ x from test limit 2;
> {code}
> If one calls to SqlString() on the node:
>  
>  
> {code:java}
> node.toSqlString(CalciteSqlDialect.DEFAULT).toString()
> {code}
>  
> We get:
>  
> {code:java}
> 'SELECT /*+ ("cpu") */ "x" FROM "test" FETCH NEXT 2 ROWS ONLY'
> {code}
> {{which is not valid SQL for a hint. It looks like the precedence options 
> need to be modified when writing out the hint in SqlSelectOperator.unparse}}
>  
> We do some rewriting of the sql statement and then re-parse it, so this is 
> preventing us from using hints. I am happy to submit a patch if I am headed 
> in the right direction with the description above. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3973) Hints should not unparse as enclosed in parentheses

2020-05-06 Thread Danny Chen (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Danny Chen updated CALCITE-3973:

Summary: Hints should not unparse as enclosed in parentheses  (was: Writing 
SQL hints to a string results in SQL that cannot be parsed )

> Hints should not unparse as enclosed in parentheses
> ---
>
> Key: CALCITE-3973
> URL: https://issues.apache.org/jira/browse/CALCITE-3973
> Project: Calcite
>  Issue Type: Bug
>Affects Versions: 1.22.0
>Reporter: Alex Baden
>Priority: Minor
> Fix For: 1.23.0
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When using the new SQL hints feature:
>  
> {code:java}
> select /*+ cpu */ x from test limit 2;
> {code}
> If one calls to SqlString() on the node:
>  
>  
> {code:java}
> node.toSqlString(CalciteSqlDialect.DEFAULT).toString()
> {code}
>  
> We get:
>  
> {code:java}
> 'SELECT /*+ ("cpu") */ "x" FROM "test" FETCH NEXT 2 ROWS ONLY'
> {code}
> {{which is not valid SQL for a hint. It looks like the precedence options 
> need to be modified when writing out the hint in SqlSelectOperator.unparse}}
>  
> We do some rewriting of the sql statement and then re-parse it, so this is 
> preventing us from using hints. I am happy to submit a patch if I am headed 
> in the right direction with the description above. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3896) Pass through parent trait requests to child operators

2020-05-06 Thread Haisheng Yuan (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101384#comment-17101384
 ] 

Haisheng Yuan commented on CALCITE-3896:


Jira can't link to github PR at the moment. Here is the PR: 
[https://github.com/apache/calcite/pull/1953]

> Pass through parent trait requests to child operators
> -
>
> Key: CALCITE-3896
> URL: https://issues.apache.org/jira/browse/CALCITE-3896
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Haisheng Yuan
>Priority: Major
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> This is not on-demand trait requests as described in [mailing 
> list|http://mail-archives.apache.org/mod_mbox/calcite-dev/201910.mbox/%3cd75b20f4-542a-4a73-897e-66ab426494c1.h.y...@alibaba-inc.com%3e],
>  which requires the overhaul of the core planner. This ticket tries to enable 
> VolcanoPlanner with basic and minimal ability to pass through parent trait 
> request to child operators without rules, though may not be flexible or 
> powerful, but should be able to work with current Calcite application with 
> minimal changes.
> The method for physical operators to implement would be:
> {code:java}
> interface RelNode {
>   RelNode passThrough(RelTraitSet required);
> }
> {code}
> Given that Calcite's physical operators decides its child operators' traits 
> when the physical operator is created in physical implementation rule, there 
> are some drawback that can't be avoided. e.g., given the following plan:
> {code:java}
> StreamAgg on [a]
>+-- MergeJoin on [a, b, c]
>|--- TableScan foo
>+--- TableScan bar
> {code}
> Suppose the MergeJoin implementation rule generates several mergejoins that 
> distributes by [a], [a,b], [a,b,c] separately. Then we pass parent operator 
> StreamAgg's trait request to MergeJoin. Since MergeJoin[a] satisfies parent's 
> request, nothing to do. Next pass request to MergeJoin[a,b], we get 
> MergeJoin[a], then pass request to MergeJoin[a,b,c], we get MergeJoin[a] 
> again. We know they are redundant and there is no need to pass through parent 
> operator's trait request, but these MergeJoin operators are independent and 
> agnostic of each other's existence.
> The ideal way is that in physical implementation rule, during the creation of 
> physical operator, it should not care about itself and its child operators' 
> physical traits. But this is another different topic.
> Anyway, better than nothing, once it is done, we can provide the option to 
> obsolete or disable  {{AbstractConverter}}, but still be able to do property 
> enforcement. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (CALCITE-3965) Excessive time waiting on DiffRepository lock

2020-05-06 Thread Laurent Goujon (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Laurent Goujon resolved CALCITE-3965.
-
Fix Version/s: 1.23.0
   Resolution: Fixed

Changed merged as 
[0220166|https://github.com/apache/calcite/commit/022016611091fdc021686aaf1a44f5217b4b3d30]

> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized 
> and gradle/junit tries to use as many cores as possible (16 on my machine). 
> But the tests take a very long time, approximatevely 90minutes on my machine, 
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that 
> most of them are waiting on {{DiffRepository}} methods 
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained 
> the lock (and is usually flushing data on disk).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Xiening Dai (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101260#comment-17101260
 ] 

Xiening Dai commented on CALCITE-3972:
--

Tried push/pop approach. It's a little tricky to implement. The push() would 
have to call Convention#transformRelBuilder, but pop() would directly pop the 
RelFactory stack. I can see some inconsistency here. On the other hand, unlike 
the RelNode stack, convention doesn't really need to maintain the previous 
status, which also makes it easier to jump from one to another (without having 
to pop() multiple times).

I keep adoptConvention() interface. Latest PR is here - 
https://github.com/apache/calcite/pull/1884/files 

Thanks.

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3975) ProjectFilterTransposeRule should succeed for project that happens to reference all input columns

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3975?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101238#comment-17101238
 ] 

Julian Hyde commented on CALCITE-3975:
--

I surmise there are a couple of things going on:
* If all of the columns are projected, then pushing the project is valid but is 
probably not a win from a cost standpoint.
* The rule needs to avoid firing on the project that is added to permute 
columns back into their original order. A permutation will project all columns, 
checking for all columns being projected suffices.

I am guessing that you want to push down for reasons other than cost. If so we 
would need a variant of this rule, and it would need a more precise check for 
permutations.  {{Project.getPermutation()}} would do it.

> ProjectFilterTransposeRule should succeed for project that happens to 
> reference all input columns
> -
>
> Key: CALCITE-3975
> URL: https://issues.apache.org/jira/browse/CALCITE-3975
> Project: Calcite
>  Issue Type: Bug
>Reporter: Steven Talbot
>Priority: Major
>
> ... that is, I think
> If I make the trivial fix of just "only skip trivial projects", something 
> like 
> {noformat}
> && origProj.getProjects().stream().allMatch((proj) -> proj instanceof 
> RexInputRef) {noformat}
> at 
> [https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354
>  
> |https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354]HepPlanner
>  goes into infinite recursion with the rule.
> But here's the test case:
>  
> {code:java}
> @Test public void testPushProjectPastFilter3() {
>   final String sql = "select empno + deptno, ename, job, mgr, hiredate, sal, 
> comm, slacker from emp where sal = 10 * comm\n"
>   + "and upper(ename) = 'FOO'";
>   sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
> }
> {code}
>  
>  
>  
> {noformat}
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> {noformat}
>  
> There's no reason the rule shouldn't succeed here, right? Or am I missing 
> something?
> The reason this rule goes into an infinite recursion with hepplanner is 
> because it sticks a project on top after transpose to handle common 
> expressions extracted from the filter and the project. Ideally, it could have 
> a mode where it could avoid doing that and do a true "transpose" if there was 
> no need for it. For example, I don't think there is a a need for a reproject 
> on top in this test case: you can just transpose and everything works as it 
> should. This would be another way to avoid infinite recursion.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3975) ProjectFilterTransposeRule should succeed for project that happens to reference all input columns

2020-05-06 Thread Steven Talbot (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3975?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Steven Talbot updated CALCITE-3975:
---
Description: 
... that is, I think

If I make the trivial fix of just "only skip trivial projects", something like 
{noformat}
&& origProj.getProjects().stream().allMatch((proj) -> proj instanceof 
RexInputRef) {noformat}
at 
[https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354
 
|https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354]HepPlanner
 goes into infinite recursion with the rule.

But here's the test case:

 
{code:java}
@Test public void testPushProjectPastFilter3() {
  final String sql = "select empno + deptno, ename, job, mgr, hiredate, sal, 
comm, slacker from emp where sal = 10 * comm\n"
  + "and upper(ename) = 'FOO'";
  sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
}
{code}
 

 

 
{noformat}










{noformat}
 

There's no reason the rule shouldn't succeed here, right? Or am I missing 
something?

The reason this rule goes into an infinite recursion with hepplanner is because 
it sticks a project on top after transpose to handle common expressions 
extracted from the filter and the project. Ideally, it could have a mode where 
it could avoid doing that and do a true "transpose" if there was no need for 
it. For example, I don't think there is a a need for a reproject on top in this 
test case: you can just transpose and everything works as it should. This would 
be another way to avoid infinite recursion.

  was:
... that is, I think

If I make the trivial fix of just "only skip trivial projects", something like 
{noformat}
&& origProj.getProjects().stream().allMatch((proj) -> proj instanceof 
RexInputRef) {noformat}
at 
[https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354
 
|https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354]HepPlanner
 goes into infinite recursion with the rule.

But here's the test case:

 
{code:java}
@Test public void testPushProjectPastFilter3() {
  final String sql = "select empno + deptno, ename, job, mgr, hiredate, sal, 
comm, slacker from emp where sal = 10 * comm\n"
  + "and upper(ename) = 'FOO'";
  sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
}
{code}
 

 

 
{noformat}










{noformat}
 

There's no reason the rule shouldn't fire here, right? Or am I missing 
something?

The reason this rule goes into an infinite recursion with hepplanner is because 
it sticks a project on top after transpose to handle common expressions 
extracted from the filter and the project. Ideally, it could have a mode where 
it could avoid doing that and do a true "transpose" if there was no need for 
it. For example, I don't think there is a a need for a reproject on top in this 
test case: you can just transpose and everything works as it should. This would 
be another way to avoid infinite recursion.


> ProjectFilterTransposeRule should succeed for project that happens to 
> reference all input columns
> -
>
> Key: CALCITE-3975
> URL: https://issues.apache.org/jira/browse/CALCITE-3975
> Project: Calcite
>  Issue Type: Bug
>Reporter: Steven Talbot
>Priority: Major
>
> ... that is, I think
> If I make the trivial fix of just "only skip trivial projects", something 
> like 
> {noformat}
> && origProj.getProjects().stream().allMatch((proj) -> proj instanceof 
> RexInputRef) {noformat}
> at 
> [https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354
>  
> |https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354]HepPlanner
>  goes into infinite recursion with the rule.
> But here's the test case:
>  
> {code:java}
> @Test public void testPushProjectPastFilter3() {
>   final String sql = "select empno + deptno, ename, job, mgr, hiredate, sal, 
> comm, slacker from emp where sal = 10 * comm\n"
>   + "and upper(ename) = 'FOO'";
>   sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
> }
> {code}
>  
>  
>  
> {noformat}
> 
> 
> 
> 
> 
> 
>  

[jira] [Updated] (CALCITE-3975) ProjectFilterTransposeRule should succeed for project that happens to reference all input columns

2020-05-06 Thread Steven Talbot (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3975?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Steven Talbot updated CALCITE-3975:
---
Summary: ProjectFilterTransposeRule should succeed for project that happens 
to reference all input columns  (was: ProjectFilterTransposeRule should fire 
for project that happens to reference all input columns)

> ProjectFilterTransposeRule should succeed for project that happens to 
> reference all input columns
> -
>
> Key: CALCITE-3975
> URL: https://issues.apache.org/jira/browse/CALCITE-3975
> Project: Calcite
>  Issue Type: Bug
>Reporter: Steven Talbot
>Priority: Major
>
> ... that is, I think
> If I make the trivial fix of just "only skip trivial projects", something 
> like 
> {noformat}
> && origProj.getProjects().stream().allMatch((proj) -> proj instanceof 
> RexInputRef) {noformat}
> at 
> [https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354
>  
> |https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354]HepPlanner
>  goes into infinite recursion with the rule.
> But here's the test case:
>  
> {code:java}
> @Test public void testPushProjectPastFilter3() {
>   final String sql = "select empno + deptno, ename, job, mgr, hiredate, sal, 
> comm, slacker from emp where sal = 10 * comm\n"
>   + "and upper(ename) = 'FOO'";
>   sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
> }
> {code}
>  
>  
>  
> {noformat}
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> {noformat}
>  
> There's no reason the rule shouldn't fire here, right? Or am I missing 
> something?
> The reason this rule goes into an infinite recursion with hepplanner is 
> because it sticks a project on top after transpose to handle common 
> expressions extracted from the filter and the project. Ideally, it could have 
> a mode where it could avoid doing that and do a true "transpose" if there was 
> no need for it. For example, I don't think there is a a need for a reproject 
> on top in this test case: you can just transpose and everything works as it 
> should. This would be another way to avoid infinite recursion.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (CALCITE-3975) ProjectFilterTransposeRule should fire for project that happens to reference all input columns

2020-05-06 Thread Steven Talbot (Jira)
Steven Talbot created CALCITE-3975:
--

 Summary: ProjectFilterTransposeRule should fire for project that 
happens to reference all input columns
 Key: CALCITE-3975
 URL: https://issues.apache.org/jira/browse/CALCITE-3975
 Project: Calcite
  Issue Type: Bug
Reporter: Steven Talbot


... that is, I think

If I make the trivial fix of just "only skip trivial projects", something like 
{noformat}
&& origProj.getProjects().stream().allMatch((proj) -> proj instanceof 
RexInputRef) {noformat}
at 
[https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354
 
|https://github.com/apache/calcite/blob/571731b80a58eb095ebac7123285c375e7afff90/core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java#L354]HepPlanner
 goes into infinite recursion with the rule.

But here's the test case:

 
{code:java}
@Test public void testPushProjectPastFilter3() {
  final String sql = "select empno + deptno, ename, job, mgr, hiredate, sal, 
comm, slacker from emp where sal = 10 * comm\n"
  + "and upper(ename) = 'FOO'";
  sql(sql).withRule(ProjectFilterTransposeRule.INSTANCE).check();
}
{code}
 

 

 
{noformat}










{noformat}
 

There's no reason the rule shouldn't fire here, right? Or am I missing 
something?

The reason this rule goes into an infinite recursion with hepplanner is because 
it sticks a project on top after transpose to handle common expressions 
extracted from the filter and the project. Ideally, it could have a mode where 
it could avoid doing that and do a true "transpose" if there was no need for 
it. For example, I don't think there is a a need for a reproject on top in this 
test case: you can just transpose and everything works as it should. This would 
be another way to avoid infinite recursion.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Xiening Dai (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101221#comment-17101221
 ] 

Xiening Dai commented on CALCITE-3972:
--

[~hyuan] I also gave an example in CALCITE-2970.

[~julianhyde] I lean towards keeping convention as part of the mutable state so 
the example would just work. Otherwise we would have to break up the steps. I 
can try 'pop/push' syntax instead.

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101205#comment-17101205
 ] 

Julian Hyde commented on CALCITE-3972:
--

RelBuilder isn't just used in rules. It is used for larger tasks, like 
SqlToRelConverter. And it is the recommended translator if someone has built a 
front end based on their own query language. We want RelBuilder's lifecycle to 
be simple and predictable when translating a complex query into dozens or 
hundreds of RelNodes. There might be multiple conventions in such queries.

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock

2020-05-06 Thread Laurent Goujon (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101202#comment-17101202
 ] 

Laurent Goujon commented on CALCITE-3965:
-

Sounds reasonable

> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized 
> and gradle/junit tries to use as many cores as possible (16 on my machine). 
> But the tests take a very long time, approximatevely 90minutes on my machine, 
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that 
> most of them are waiting on {{DiffRepository}} methods 
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained 
> the lock (and is usually flushing data on disk).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3974) Add some accessors for Calcite AST nodes private data fields

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3974?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101199#comment-17101199
 ] 

Julian Hyde commented on CALCITE-3974:
--

For data fields the style tends to be to make the fields public and final 
rather than add accessor methods.

Rationale: people don't tend to document accessor methods very well; over time 
the SqlNode AST classes are becoming mostly immutable, so don't need 'set' 
methods. There are a few 'virtual' accessors, for example 
{{SqlCall.getOperator()}}, but they are exceptions.

> Add some accessors for Calcite AST nodes private data fields
> 
>
> Key: CALCITE-3974
> URL: https://issues.apache.org/jira/browse/CALCITE-3974
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Drew Schmitt
>Priority: Minor
>
> There are a few private fields in Calcite's AST nodes that would be easier to 
> access with an explicit getter, rather than using the getOperandList() method.
> For example, SqlCreateFunction's name field is private, but can already be 
> access through getOperandList().get(0). We should add an explicit getName() 
> getter.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101195#comment-17101195
 ] 

Julian Hyde commented on CALCITE-3965:
--

Thanks for fixing both this and CALCITE-3517. Both PRs look good. (I haven't 
run them.)

How about merging this PR ({{synchronized}}) first, and let it run in CI for 2 
or 3 days before merging the other PR? Just in case it causes some instability.

> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized 
> and gradle/junit tries to use as many cores as possible (16 on my machine). 
> But the tests take a very long time, approximatevely 90minutes on my machine, 
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that 
> most of them are waiting on {{DiffRepository}} methods 
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained 
> the lock (and is usually flushing data on disk).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (CALCITE-3887) Filter and Join conditions may not need to retain nullability during simplifications

2020-05-06 Thread Zoltan Haindrich (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3887?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Zoltan Haindrich resolved CALCITE-3887.
---
Fix Version/s: 1.23.0
   Resolution: Fixed

> Filter and Join conditions may not need to retain nullability during 
> simplifications
> 
>
> Key: CALCITE-3887
> URL: https://issues.apache.org/jira/browse/CALCITE-3887
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Zoltan Haindrich
>Assignee: Zoltan Haindrich
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.23.0
>
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> Reported by [~icshuo]: that there are situation in which join conditions are 
> placed inside a redundant `CAST`.
> https://mail-archives.apache.org/mod_mbox/calcite-dev/202003.mbox/%3CCABYySnvtrMFWaBqM7_FgKedXELdXeCqciRJV7G4MyYN07HUOnQ%40mail.gmail.com%3E



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3887) Filter and Join conditions may not need to retain nullability during simplifications

2020-05-06 Thread Zoltan Haindrich (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101193#comment-17101193
 ] 

Zoltan Haindrich commented on CALCITE-3887:
---

Fixed in 
[cecfece459b0f5f155c0498811985e083813aa20|https://github.com/apache/calcite/commit/cecfece459b0f5f155c0498811985e083813aa20]

> Filter and Join conditions may not need to retain nullability during 
> simplifications
> 
>
> Key: CALCITE-3887
> URL: https://issues.apache.org/jira/browse/CALCITE-3887
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Zoltan Haindrich
>Assignee: Zoltan Haindrich
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> Reported by [~icshuo]: that there are situation in which join conditions are 
> placed inside a redundant `CAST`.
> https://mail-archives.apache.org/mod_mbox/calcite-dev/202003.mbox/%3CCABYySnvtrMFWaBqM7_FgKedXELdXeCqciRJV7G4MyYN07HUOnQ%40mail.gmail.com%3E



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3974) Add some accessors for Calcite AST nodes private data fields

2020-05-06 Thread Drew Schmitt (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3974?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Drew Schmitt updated CALCITE-3974:
--
Priority: Minor  (was: Major)

> Add some accessors for Calcite AST nodes private data fields
> 
>
> Key: CALCITE-3974
> URL: https://issues.apache.org/jira/browse/CALCITE-3974
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Drew Schmitt
>Priority: Minor
>
> There are a few private fields in Calcite's AST nodes that would be easier to 
> access with an explicit getter, rather than using the getOperandList() method.
> For example, SqlCreateFunction's name field is private, but can already be 
> access through getOperandList().get(0). We should add an explicit getName() 
> getter.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (CALCITE-3974) Add some accessors for Calcite AST nodes private data fields

2020-05-06 Thread Drew Schmitt (Jira)
Drew Schmitt created CALCITE-3974:
-

 Summary: Add some accessors for Calcite AST nodes private data 
fields
 Key: CALCITE-3974
 URL: https://issues.apache.org/jira/browse/CALCITE-3974
 Project: Calcite
  Issue Type: Improvement
Reporter: Drew Schmitt


There are a few private fields in Calcite's AST nodes that would be easier to 
access with an explicit getter, rather than using the getOperandList() method.

For example, SqlCreateFunction's name field is private, but can already be 
access through getOperandList().get(0). We should add an explicit getName() 
getter.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Haisheng Yuan (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101186#comment-17101186
 ] 

Haisheng Yuan commented on CALCITE-3972:


I am curious when do we need to preserve the stack while switching convention. 
Even in ProjectMergeRule, there is only 1 convention needed for the RelBuilder 
for each rule execution. It is either logical or physical RelBuilder. We won't 
need a RelBuilder for 2 conventions in the same rule instance at the same time.

I think I am missing something. Any use cases?

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3517) DiffRepository spends too much time writing XML, makes some tests 5x slower

2020-05-06 Thread Laurent Goujon (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101182#comment-17101182
 ] 

Laurent Goujon commented on CALCITE-3517:
-

I'm proposing a patch to create a JUnit extension to manage {{DiffRepository}} 
lifecycle so that the file is only dumped once all the tests are run: 
https://github.com/apache/calcite/pull/1962

> DiffRepository spends too much time writing XML, makes some tests 5x slower
> ---
>
> Key: CALCITE-3517
> URL: https://issues.apache.org/jira/browse/CALCITE-3517
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Priority: Major
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Tests that use {{DiffRepository}} are spending far too much effort writing 
> XML, even if the XML matches the reference file. For example, If I comment 
> out [a call to set(tag, 
> next)|https://github.com/apache/calcite/blob/ee83efd360793ef4201f4cdfc2af8d837b76ca69/core/src/test/java/org/apache/calcite/test/DiffRepository.java#L267],
>  {{RelOptRulesTest}} improves from 32s to 6s; {{SqlToRelConverterTest}} 
> improves from 24s to 4.7s; {{SqlPrettyWriterTest}} remains .8s.
> The {{DiffRepository.expand}} method is the cause of the inefficiency. It 
> causes the entire XML document to be re-generated and written to disk. This 
> is not just slow but quadratic - if a test has N cases, each test writes the 
> XML document, an effort proportional to N.
> {{DiffRepository}} should remain conservative. If one of the tests fails, and 
> a later test crashes, the output from the failed test should have been 
> written out. It is acceptable if the test remains slow if there are test 
> failures.
> {{DiffRepository}} is only used in tests; this bug does not affect production 
> code.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101170#comment-17101170
 ] 

Julian Hyde commented on CALCITE-3972:
--

I don't mind if {{RelBuilder#transform(UnaryOperator transform)}} - or 
a variant of it - preserves the stack. I planned to go in that direction anyway.

I agree that, whatever we decide for the semantics of {{adoptConvention}}, your 
example {code}RelNode root =
builder.adoptConvention(EnumerableConvention.INSTANCE)
 .scan("EMP")
.filter(
builder.equals(builder.field("DEPTNO"), builder.literal(20)))
.build();{code} should work.

If we're going to make convention part of the mutable state, perhaps a 
'push'/'pop' model is better than a 'set' model. Then it's easier to get back 
to the previous convention.

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3965) Excessive time waiting on DiffRepository lock

2020-05-06 Thread Laurent Goujon (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101161#comment-17101161
 ] 

Laurent Goujon commented on CALCITE-3965:
-

I guess after all this time, I still don't know how the JIRA/Github integration 
works: https://github.com/apache/calcite/pull/1954

> Excessive time waiting on DiffRepository lock
> -
>
> Key: CALCITE-3965
> URL: https://issues.apache.org/jira/browse/CALCITE-3965
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Reporter: Laurent Goujon
>Assignee: Laurent Goujon
>Priority: Major
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When running the whole test suite from commandline, tests are parallelized 
> and gradle/junit tries to use as many cores as possible (16 on my machine). 
> But the tests take a very long time, approximatevely 90minutes on my machine, 
> and several of them failed because they took too long to complete.
> Using jstack to look at the threads state while tests are running show that 
> most of them are waiting on {{DiffRepository}} methods 
> ({{DiffRepository#expand}} in most cases) while one of the thread obtained 
> the lock (and is usually flushing data on disk).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Xiening Dai (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101144#comment-17101144
 ] 

Xiening Dai commented on CALCITE-3972:
--

If convention is considered as part of the config, then it looks weird that 
adoptConvention() preserves builder stack while 
RelBuilder#transform(UnaryOperator transform) does not. 

Furthermore, a common usage pattern like below won't work because the builder 
object referenced by filter is not the one after adoptConvention() -

{code:java}
RelNode root =
builder.adoptConvention(EnumerableConvention.INSTANCE)
 .scan("EMP")
.filter(
builder.equals(builder.field("DEPTNO"), builder.literal(20)))
.build();
{code}

In my view, if we'd like to support switching convention in the middle of 
building nodes, we should treat convention as a mutable state - just like 
stake. Or we abandon such capability, and don't preserve stake when switching 
convention. In that case, we don't need adoptConvention(),   modifying current 
transform() should be enough.

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-2798) Optimizer should remove ORDER BY in sub-query, provided it has no LIMIT or OFFSET

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-2798?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101140#comment-17101140
 ] 

Julian Hyde commented on CALCITE-2798:
--

I think the question is: if the {{ORDER BY}} is retained, will it affect the 
results? For example,
{code}SELECT *
FROM emp
JOIN (SELECT * FROM dept ORDER BY deptno) USING (deptno){code}
is not guaranteed to produce results sorted by {{deptno}} and therefore the 
optimizer should remove {{ORDER BY deptno}}. There are only two "relational 
operators" I know where order matters - user-defined table functions and a user 
viewing results of a query.

> Optimizer should remove ORDER BY in sub-query, provided it has no LIMIT or 
> OFFSET
> -
>
> Key: CALCITE-2798
> URL: https://issues.apache.org/jira/browse/CALCITE-2798
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.18.0
>Reporter: Vladimir Sitnikov
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.19.0
>
>
> The following SQL performs sort twice, however inner sort can be eliminated
> {code}select * from (
>   select * from "emps" 
> order by "emps"."deptno"
> ) order by 1 desc{code}
> The same goes for (window calculation will sort on its own)
> {code}select row_number() over (order by "emps"."deptno")  from (
>   select * from "emps" 
> order by "emps"."deptno" desc
> ){code}
> The same goes for SetOp (union, minus):
> {code}select * from (
>   select * from "emps" 
> order by "emps"."deptno"
> ) union select * from (
>   select * from "emps" 
> order by "emps"."deptno" desc
> ){code}
> There might be other cases like that (e.g. Aggregate, Join, Exchange, 
> SortExchange)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-2798) Optimizer should remove ORDER BY in sub-query, provided it has no LIMIT or OFFSET

2020-05-06 Thread Stamatis Zampetakis (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-2798?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101093#comment-17101093
 ] 

Stamatis Zampetakis commented on CALCITE-2798:
--

First apologies for commenting on a closed issue but I have the impression that 
maybe we should rethink the decision of removing the ORDER BY from sub-queries. 
If I am not interpreting wrong the SQL standard ORDER BY in sub-queries is an 
optional feature (i.e., F851) and databases like Postgres and Oracle do support 
it. 

> Optimizer should remove ORDER BY in sub-query, provided it has no LIMIT or 
> OFFSET
> -
>
> Key: CALCITE-2798
> URL: https://issues.apache.org/jira/browse/CALCITE-2798
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.18.0
>Reporter: Vladimir Sitnikov
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.19.0
>
>
> The following SQL performs sort twice, however inner sort can be eliminated
> {code}select * from (
>   select * from "emps" 
> order by "emps"."deptno"
> ) order by 1 desc{code}
> The same goes for (window calculation will sort on its own)
> {code}select row_number() over (order by "emps"."deptno")  from (
>   select * from "emps" 
> order by "emps"."deptno" desc
> ){code}
> The same goes for SetOp (union, minus):
> {code}select * from (
>   select * from "emps" 
> order by "emps"."deptno"
> ) union select * from (
>   select * from "emps" 
> order by "emps"."deptno" desc
> ){code}
> There might be other cases like that (e.g. Aggregate, Join, Exchange, 
> SortExchange)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3679) Allow lambda expressions in SQL queries

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101070#comment-17101070
 ] 

Julian Hyde commented on CALCITE-3679:
--

First impression is that the PR is rather scruffy. Comments have been 
copy-pasted without being changed. Lines of whitespace have been added and 
removed. Some classes have no javadoc.

Justify why we need a new class of RexSlot.

This case has no real description, so there's no indication of the intended 
scope. Do we intend to parse, validate, execute?

Lambdas in functional programming languages are functions-as-values. They are 
typed, they allow closures (carrying their environment for later evaluation) 
and they can be used anywhere that any other value is used. They are beautiful 
and very powerful. So a half-hearted implementation would make me very sad.

The SQL implementations I have seen (Presto and SparkSQL) have a few built-in 
higher-order functions (i.e. functions that take lambdas as arguments and/or 
return lambdas as results) but do not allow user-defined higher-order 
functions. This makes me sad.

A minimal level of functionality for Calcite would include support for lambdas 
as standalone values (e.g. 'select empno, x -> x + 1 from emp') and adequate 
typing of lambdas (not 'LAMBDA' but 'FUNCTION' or similar).

> Allow lambda expressions in SQL queries
> ---
>
> Key: CALCITE-3679
> URL: https://issues.apache.org/jira/browse/CALCITE-3679
> Project: Calcite
>  Issue Type: New Feature
>Reporter: Ritesh
>Assignee: Ritesh
>Priority: Major
>  Labels: pull-request-available
> Attachments: [CALCITE-3679]_Basic_implementation.patch
>
>  Time Spent: 5h 40m
>  Remaining Estimate: 0h
>
> [https://teradata.github.io/presto/docs/0.167-t/functions/lambda.html]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3973) Writing SQL hints to a string results in SQL that cannot be parsed

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101063#comment-17101063
 ] 

Julian Hyde commented on CALCITE-3973:
--

Let's get the PR into 1.23 if it's good.

> Writing SQL hints to a string results in SQL that cannot be parsed 
> ---
>
> Key: CALCITE-3973
> URL: https://issues.apache.org/jira/browse/CALCITE-3973
> Project: Calcite
>  Issue Type: Bug
>Affects Versions: 1.22.0
>Reporter: Alex Baden
>Priority: Minor
> Fix For: 1.23.0
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When using the new SQL hints feature:
>  
> {code:java}
> select /*+ cpu */ x from test limit 2;
> {code}
> If one calls to SqlString() on the node:
>  
>  
> {code:java}
> node.toSqlString(CalciteSqlDialect.DEFAULT).toString()
> {code}
>  
> We get:
>  
> {code:java}
> 'SELECT /*+ ("cpu") */ "x" FROM "test" FETCH NEXT 2 ROWS ONLY'
> {code}
> {{which is not valid SQL for a hint. It looks like the precedence options 
> need to be modified when writing out the hint in SqlSelectOperator.unparse}}
>  
> We do some rewriting of the sql statement and then re-parse it, so this is 
> preventing us from using hints. I am happy to submit a patch if I am headed 
> in the right direction with the description above. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3973) Writing SQL hints to a string results in SQL that cannot be parsed

2020-05-06 Thread Julian Hyde (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julian Hyde updated CALCITE-3973:
-
Affects Version/s: 1.22.0

> Writing SQL hints to a string results in SQL that cannot be parsed 
> ---
>
> Key: CALCITE-3973
> URL: https://issues.apache.org/jira/browse/CALCITE-3973
> Project: Calcite
>  Issue Type: Bug
>Affects Versions: 1.22.0
>Reporter: Alex Baden
>Priority: Minor
> Fix For: 1.23.0
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When using the new SQL hints feature:
>  
> {code:java}
> select /*+ cpu */ x from test limit 2;
> {code}
> If one calls to SqlString() on the node:
>  
>  
> {code:java}
> node.toSqlString(CalciteSqlDialect.DEFAULT).toString()
> {code}
>  
> We get:
>  
> {code:java}
> 'SELECT /*+ ("cpu") */ "x" FROM "test" FETCH NEXT 2 ROWS ONLY'
> {code}
> {{which is not valid SQL for a hint. It looks like the precedence options 
> need to be modified when writing out the hint in SqlSelectOperator.unparse}}
>  
> We do some rewriting of the sql statement and then re-parse it, so this is 
> preventing us from using hints. I am happy to submit a patch if I am headed 
> in the right direction with the description above. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3973) Writing SQL hints to a string results in SQL that cannot be parsed

2020-05-06 Thread Julian Hyde (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3973?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julian Hyde updated CALCITE-3973:
-
Fix Version/s: 1.23.0

> Writing SQL hints to a string results in SQL that cannot be parsed 
> ---
>
> Key: CALCITE-3973
> URL: https://issues.apache.org/jira/browse/CALCITE-3973
> Project: Calcite
>  Issue Type: Bug
>Reporter: Alex Baden
>Priority: Minor
> Fix For: 1.23.0
>
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When using the new SQL hints feature:
>  
> {code:java}
> select /*+ cpu */ x from test limit 2;
> {code}
> If one calls to SqlString() on the node:
>  
>  
> {code:java}
> node.toSqlString(CalciteSqlDialect.DEFAULT).toString()
> {code}
>  
> We get:
>  
> {code:java}
> 'SELECT /*+ ("cpu") */ "x" FROM "test" FETCH NEXT 2 ROWS ONLY'
> {code}
> {{which is not valid SQL for a hint. It looks like the precedence options 
> need to be modified when writing out the hint in SqlSelectOperator.unparse}}
>  
> We do some rewriting of the sql statement and then re-parse it, so this is 
> preventing us from using hints. I am happy to submit a patch if I am headed 
> in the right direction with the description above. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101043#comment-17101043
 ] 

Julian Hyde commented on CALCITE-3972:
--

[~xndai] wrote:

bq. Also the RelBuilder is not immutable today. The stack can change after it's 
passed into a function. So it's not a new problem...

I acknowledged that earlier in the comment thread. The stack of {{RelNode}}s 
(and ancillary data like aliases) is mutable. The config is not mutable. This 
is intentional - I consider config and the rel stack to be different classes of 
state.

Any class called 'builder' of course has mutable state. Builders are useful 
because they allow mutation in a controlled way. 

bq. I mean with the adoptConvention() we create a new builder every time. It 
could be problematic if this gets call frequently (such in high frequent rules).

Creating a {{RelBuilder}} is fairly efficient - 7 fields to assign, two object 
creations ({{ArrayDeque}} and {{RexSimplify}}). A typical planning process 
might fire 10k rules. 10k object creations is not going to move the dial.

> Allow RelBuilder to create RelNode with convention and use it for trait 
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
>  Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing 
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the 
> underlying RelFactories and create a new builder. 
> 3. Use the new interface in RelCollationTraitDef for converting into 
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3970) Table-valued function TUMBLE uses non-standard syntax

2020-05-06 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101029#comment-17101029
 ] 

Julian Hyde commented on CALCITE-3970:
--

I don't recall where {{CURSOR}} came from, or whether it is even in the 
standard, but it seems that this use of {{TABLE}} is very similar to Calcite's 
current use of {{CURSOR}}. So, how about allowing {{TABLE}} as an alternative 
to {{CURSOR}}, and deprecating {{CURSOR}} with an eye to eventually removing it?

> Table-valued function TUMBLE uses non-standard syntax
> -
>
> Key: CALCITE-3970
> URL: https://issues.apache.org/jira/browse/CALCITE-3970
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Affects Versions: 1.22.0
>Reporter: Viliam Durina
>Priority: Major
>
> The currently supported syntax is this:
> {code:java}
> SELECT * FROM TABLE(TUMBLE(TABLE my_table, ...
> {code}
> But the SQL standard specifies that {{my_table}} must be in parentheses, such 
> as here:
> {code:java}
> SELECT * FROM TABLE(TUMBLE(TABLE(my_table), ...
> {code}
> The second syntax is currently rejected with:
> {code:none}
> Exception in thread "main" org.apache.calcite.sql.parser.SqlParseException: 
> Encountered "(" at line 1, column 33.
> Was expecting one of:
>  ...
>  ...
>  ...
>  ...
>  ...
> {code}
> I'm not sure if the currently supported syntax is optional, but I think it's 
> not.
> I followed this document: 
> [http://standards.iso.org/ittf/PubliclyAvailableStandards/c069776_ISO_IEC_TR_19075-7_2017.zip]
> The {{TABLE}} clause acts in both ways: (1) to convert a table value to a 
> table object (when used in the {{FROM}} clause to convert the function 
> result) and (2) to convert a table object to a table value (when used to 
> convert arguments to a function).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3887) Filter and Join conditions may not need to retain nullability during simplifications

2020-05-06 Thread Zoltan Haindrich (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100799#comment-17100799
 ] 

Zoltan Haindrich commented on CALCITE-3887:
---

RelBuilder was already doing this by calling the right methods - I've added 
some tests.

> Filter and Join conditions may not need to retain nullability during 
> simplifications
> 
>
> Key: CALCITE-3887
> URL: https://issues.apache.org/jira/browse/CALCITE-3887
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Zoltan Haindrich
>Assignee: Zoltan Haindrich
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> Reported by [~icshuo]: that there are situation in which join conditions are 
> placed inside a redundant `CAST`.
> https://mail-archives.apache.org/mod_mbox/calcite-dev/202003.mbox/%3CCABYySnvtrMFWaBqM7_FgKedXELdXeCqciRJV7G4MyYN07HUOnQ%40mail.gmail.com%3E



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3951) Support different string comparison based on SqlCollation

2020-05-06 Thread Ruben Q L (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ruben Q L updated CALCITE-3951:
---
Labels: pull-request-available  (was: )

> Support different string comparison based on SqlCollation
> -
>
> Key: CALCITE-3951
> URL: https://issues.apache.org/jira/browse/CALCITE-3951
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Ruben Q L
>Assignee: Ruben Q L
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 3h
>  Remaining Estimate: 0h
>
> Currently SqlCollation defines concepts like Coercibility, Charset, Locale, 
> etc. However, we cannot specify on a certain collation that e.g. a string 
> field should use case insensitive comparison. The goal of this ticket is to 
> evolve SqlCollation to support that, and adapt the corresponding classes to 
> use that (optional) "non-standard" comparison.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Comment Edited] (CALCITE-3916) Support cascades style top-down driven rule apply

2020-05-06 Thread Jinpeng Wu (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100596#comment-17100596
 ] 

Jinpeng Wu edited comment on CALCITE-3916 at 5/6/20, 9:35 AM:
--

PR: https://github.com/apache/calcite/pull/1950 

There might be two ways to accomplish this. The first one is designing another 
Planner while the second is modifying the VolcanoPlanner directly and make sure 
it won't break the current logic. The pros and cons are discussed: 
https://lists.apache.org/thread.html/r38ea71968c069f465921e7197488329c15413b46831c90ad4d48f87e%40%3Cdev.calcite.apache.org%3E
 

The code in this PR is now generally on the first track because I am still 
trying some aggressive optimization. If keeping one VolcanoPlanner is the 
consensus, it's definitely possible to combine this PR with VolcannoPlanner. 



was (Author: fatlittle):
PR: https://github.com/apache/calcite/pull/1950 

There might be two ways to accomplish this. The first one is designing another 
Planner while the second is modifying the VolcanoPlanner directly and make sure 
it won't break the current logic. The pros and cons are discussed: 
https://lists.apache.org/thread.html/r38ea71968c069f465921e7197488329c15413b46831c90ad4d48f87e%40%3Cdev.calcite.apache.org%3E
 

My code is now generally on the first track. Currently it should not be 
difficult to switch to the second one. However, I am still trying some 
aggressive optimizations. So I am not going to take the second way until many 
people insist. Thanks

> Support cascades style top-down driven rule apply
> -
>
> Key: CALCITE-3916
> URL: https://issues.apache.org/jira/browse/CALCITE-3916
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Haisheng Yuan
>Assignee: Jinpeng Wu
>Priority: Major
>
> Apply rules by leaf RelSet -> root RelSet order. For every RelNode in a 
> RelSet, rule is matched and applied sequentially. No RuleQueue and 
> DeferringRuleCall is needed anymore. This will make space pruning and rule 
> mutual exclusivity check possible.
> Rule that use AbstractConverter as operand is an exception, to keep backward 
> compatibility, this kind of rule still needs top-down apply.
> This should be done after CALCITE-3896.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3916) Support cascades style top-down driven rule apply

2020-05-06 Thread Jinpeng Wu (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100596#comment-17100596
 ] 

Jinpeng Wu commented on CALCITE-3916:
-

PR: https://github.com/apache/calcite/pull/1950 

There might be two ways to accomplish this. The first one is designing another 
Planner while the second is modifying the VolcanoPlanner directly and make sure 
it won't break the current logic. The pros and cons are discussed: 
https://lists.apache.org/thread.html/r38ea71968c069f465921e7197488329c15413b46831c90ad4d48f87e%40%3Cdev.calcite.apache.org%3E
 

My code is now generally on the first track. Currently it should not be 
difficult to switch to the second one. However, I am still trying some 
aggressive optimizations. So I am not going to take the second way until many 
people insist. Thanks

> Support cascades style top-down driven rule apply
> -
>
> Key: CALCITE-3916
> URL: https://issues.apache.org/jira/browse/CALCITE-3916
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Haisheng Yuan
>Assignee: Jinpeng Wu
>Priority: Major
>
> Apply rules by leaf RelSet -> root RelSet order. For every RelNode in a 
> RelSet, rule is matched and applied sequentially. No RuleQueue and 
> DeferringRuleCall is needed anymore. This will make space pruning and rule 
> mutual exclusivity check possible.
> Rule that use AbstractConverter as operand is an exception, to keep backward 
> compatibility, this kind of rule still needs top-down apply.
> This should be done after CALCITE-3896.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Assigned] (CALCITE-3916) Support cascades style top-down driven rule apply

2020-05-06 Thread Jinpeng Wu (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3916?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jinpeng Wu reassigned CALCITE-3916:
---

Assignee: Jinpeng Wu  (was: Haisheng Yuan)

> Support cascades style top-down driven rule apply
> -
>
> Key: CALCITE-3916
> URL: https://issues.apache.org/jira/browse/CALCITE-3916
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Haisheng Yuan
>Assignee: Jinpeng Wu
>Priority: Major
>
> Apply rules by leaf RelSet -> root RelSet order. For every RelNode in a 
> RelSet, rule is matched and applied sequentially. No RuleQueue and 
> DeferringRuleCall is needed anymore. This will make space pruning and rule 
> mutual exclusivity check possible.
> Rule that use AbstractConverter as operand is an exception, to keep backward 
> compatibility, this kind of rule still needs top-down apply.
> This should be done after CALCITE-3896.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3951) Support different string comparison based on SqlCollation

2020-05-06 Thread Stamatis Zampetakis (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3951?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100573#comment-17100573
 ] 

Stamatis Zampetakis commented on CALCITE-3951:
--

Hey [~rubenql], I am quite busy at the moment, will try to look over the 
weekend. 

> Support different string comparison based on SqlCollation
> -
>
> Key: CALCITE-3951
> URL: https://issues.apache.org/jira/browse/CALCITE-3951
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Ruben Q L
>Assignee: Ruben Q L
>Priority: Major
>  Time Spent: 3h
>  Remaining Estimate: 0h
>
> Currently SqlCollation defines concepts like Coercibility, Charset, Locale, 
> etc. However, we cannot specify on a certain collation that e.g. a string 
> field should use case insensitive comparison. The goal of this ticket is to 
> evolve SqlCollation to support that, and adapt the corresponding classes to 
> use that (optional) "non-standard" comparison.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Comment Edited] (CALCITE-3970) Table-valued function TUMBLE uses non-standard syntax

2020-05-06 Thread Viliam Durina (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100541#comment-17100541
 ] 

Viliam Durina edited comment on CALCITE-3970 at 5/6/20, 8:00 AM:
-

See the section "8.6":
{code:java}
 ::=
TABLE   
  | TABLE 
  | 
{code}
 also specifies that a subquery also uses TABLE keyword, not 
CURSOR (see section 8.6.2):
{code:java}
FROMTABLE (Ptf ( TABLE ( SELECT * FROM Emp ) ) ) AS P
{code}
 All the examples in the spec use {{TABLE(foo)}} syntax.


was (Author: vilo):
See the section "8.6":
{code:java}
 ::=
TABLE   
  | TABLE 
  | 
{code}
 also specifies that a subquery also uses TABLE keyword, not 
CURSOR (see section 8.6.2):
{code:java}
FROMTABLE (Ptf ( TABLE ( SELECT * FROM Emp ) ) ) AS P
{code}
 

> Table-valued function TUMBLE uses non-standard syntax
> -
>
> Key: CALCITE-3970
> URL: https://issues.apache.org/jira/browse/CALCITE-3970
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Affects Versions: 1.22.0
>Reporter: Viliam Durina
>Priority: Major
>
> The currently supported syntax is this:
> {code:java}
> SELECT * FROM TABLE(TUMBLE(TABLE my_table, ...
> {code}
> But the SQL standard specifies that {{my_table}} must be in parentheses, such 
> as here:
> {code:java}
> SELECT * FROM TABLE(TUMBLE(TABLE(my_table), ...
> {code}
> The second syntax is currently rejected with:
> {code:none}
> Exception in thread "main" org.apache.calcite.sql.parser.SqlParseException: 
> Encountered "(" at line 1, column 33.
> Was expecting one of:
>  ...
>  ...
>  ...
>  ...
>  ...
> {code}
> I'm not sure if the currently supported syntax is optional, but I think it's 
> not.
> I followed this document: 
> [http://standards.iso.org/ittf/PubliclyAvailableStandards/c069776_ISO_IEC_TR_19075-7_2017.zip]
> The {{TABLE}} clause acts in both ways: (1) to convert a table value to a 
> table object (when used in the {{FROM}} clause to convert the function 
> result) and (2) to convert a table object to a table value (when used to 
> convert arguments to a function).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3970) Table-valued function TUMBLE uses non-standard syntax

2020-05-06 Thread Viliam Durina (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100541#comment-17100541
 ] 

Viliam Durina commented on CALCITE-3970:


See the section "8.6":
{code:java}
 ::=
TABLE   
  | TABLE 
  | 
{code}
 also specifies that a subquery also uses TABLE keyword, not 
CURSOR (see section 8.6.2):
{code:java}
FROMTABLE (Ptf ( TABLE ( SELECT * FROM Emp ) ) ) AS P
{code}
 

> Table-valued function TUMBLE uses non-standard syntax
> -
>
> Key: CALCITE-3970
> URL: https://issues.apache.org/jira/browse/CALCITE-3970
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Affects Versions: 1.22.0
>Reporter: Viliam Durina
>Priority: Major
>
> The currently supported syntax is this:
> {code:java}
> SELECT * FROM TABLE(TUMBLE(TABLE my_table, ...
> {code}
> But the SQL standard specifies that {{my_table}} must be in parentheses, such 
> as here:
> {code:java}
> SELECT * FROM TABLE(TUMBLE(TABLE(my_table), ...
> {code}
> The second syntax is currently rejected with:
> {code:none}
> Exception in thread "main" org.apache.calcite.sql.parser.SqlParseException: 
> Encountered "(" at line 1, column 33.
> Was expecting one of:
>  ...
>  ...
>  ...
>  ...
>  ...
> {code}
> I'm not sure if the currently supported syntax is optional, but I think it's 
> not.
> I followed this document: 
> [http://standards.iso.org/ittf/PubliclyAvailableStandards/c069776_ISO_IEC_TR_19075-7_2017.zip]
> The {{TABLE}} clause acts in both ways: (1) to convert a table value to a 
> table object (when used in the {{FROM}} clause to convert the function 
> result) and (2) to convert a table object to a table value (when used to 
> convert arguments to a function).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3973) Writing SQL hints to a string results in SQL that cannot be parsed

2020-05-06 Thread Alex Baden (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100495#comment-17100495
 ] 

Alex Baden commented on CALCITE-3973:
-

Thanks [~danny0405], I've put up a PR: 
[https://github.com/apache/calcite/pull/1961]

> Writing SQL hints to a string results in SQL that cannot be parsed 
> ---
>
> Key: CALCITE-3973
> URL: https://issues.apache.org/jira/browse/CALCITE-3973
> Project: Calcite
>  Issue Type: Bug
>Reporter: Alex Baden
>Priority: Minor
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> When using the new SQL hints feature:
>  
> {code:java}
> select /*+ cpu */ x from test limit 2;
> {code}
> If one calls to SqlString() on the node:
>  
>  
> {code:java}
> node.toSqlString(CalciteSqlDialect.DEFAULT).toString()
> {code}
>  
> We get:
>  
> {code:java}
> 'SELECT /*+ ("cpu") */ "x" FROM "test" FETCH NEXT 2 ROWS ONLY'
> {code}
> {{which is not valid SQL for a hint. It looks like the precedence options 
> need to be modified when writing out the hint in SqlSelectOperator.unparse}}
>  
> We do some rewriting of the sql statement and then re-parse it, so this is 
> preventing us from using hints. I am happy to submit a patch if I am headed 
> in the right direction with the description above. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)