[jira] [Comment Edited] (CALCITE-2997) Avoid pushing down join condition in SqlToRelConverter

2020-05-20 Thread Chunwei Lei (Jira)


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

Chunwei Lei edited comment on CALCITE-2997 at 5/21/20, 2:29 AM:


Should we also move other optimization mentioned by [~jinxing6...@126.com] to 
RelBuilder too?


was (Author: chunwei lei):
I think It may be less overhead if such optimizations are done in 
SqltoRelConverter though it is a little weird. But I'm fine with the PR. Should 
we also move other optimization mentioned by [~jinxing6...@126.com] to 
RelBuilder too?

> Avoid pushing down join condition in SqlToRelConverter
> --
>
> Key: CALCITE-2997
> URL: https://issues.apache.org/jira/browse/CALCITE-2997
> Project: Calcite
>  Issue Type: Bug
>Reporter: Jin Xing
>Assignee: Julian Hyde
>Priority: Major
>
> In current code, *SqlToRelConverter:createJoin* is calling 
> *RelOptUtil.pushDownJoinConditions* for optimization. And we can find below 
> conversion from *SqlNode* to *RelNode*:
> {code:java}
> SqlNode:
> select * from A join B on A.x = B.x * 2
> RelNode (Logical-Plan):
> Join (condition:col0=col1)
> |-Project(x as col0)
> | |-Scan(A)
> |-Project(x * 2 as col1)
>   |-Scan(B){code}
> As we can see the logical plan(*RelNode*) posted above is not the pure 
> reflection of the original SQL String(*SqlNode*). The optimization is mixed 
> into the phase on which AST is converted to Logical-Plan. Actually optimizing 
> rule of JoinPushExpressionsRule is doing exactly the same kind of thing. 
> Shall we just keep the optimization inside Optimized-Logical-Plan ? I mean 
> shall we avoid calling *RelOptUtil.pushDownJoinConditions* in 
> *SqlToRelConverter:createJoin*
> I raised this issue because that we are doing something based on the 
> Logical-Plan. And it makes us really confused that the Logical-Plan doesn't 
> corresponds to SqlNode. 
>  
>  



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


[jira] [Comment Edited] (CALCITE-2997) Avoid pushing down join condition in SqlToRelConverter

2020-05-19 Thread Jin Xing (Jira)


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

Jin Xing edited comment on CALCITE-2997 at 5/20/20, 2:36 AM:
-

Thanks [~swtalbot] and [~julianhyde] to bring this up ~

Yes, In some scenarios we need SqlToRelConverter to return a logical plan which 
corresponds to original SQL String exactly.

Mixing optimizations into the process of building logical plan sometimes make 
the user confused.

I would +1 on Julian's idea – – move the rewrite/optimize code into RelBuilder 
and add a config to enable/disable, but several concerns as below:
 # Should we disable the rewrite/optimize in SqlToRelConverter by default ? 
Calcite have clear phases for the lifecycle of a SQL. SqlToRelConverter can 
return a purely reflection of the original SQL string, and let the 
optimization/rewrite done in the following optimizing phase;
 # In addition to pushDownJoinConditions, there are other rewrites like 
pushDownNotForIn, shall we disable these optimizing work by default or even 
move out of SqlToRelConverter to the optimizaing phase ?


was (Author: jinxing6...@126.com):
Thanks [~swtalbot] and [~julianhyde] to bring this up ~

Yes, In some scenarios we need SqlToRelConverter to return a logical plan which 
corresponds to original SQL String exactly.

Mixing optimizations into the process of building logical plan sometimes make 
the user confused.

I would +1 on Julian's idea – – move the rewrite/optimize code into RelBuilder 
and add a config to enable/disable, but several concerns as below:
 # Should we disable the rewrite/optimize in SqlToRelConverter by default ? 
Calcite have clear phases for the lifecycle of a SQL. SqlToRelConverter can 
return a purely reflection of the original SQL string, and let the 
optimization/rewrite done in the following optimizing phase;
 # In addition to pushDownJoinConditions, there are other rewrites like 
pushDownNotForIn, shall we disable these optimizing work or even move out of 
SqlToRelConverter to the optimizaing phase ?

> Avoid pushing down join condition in SqlToRelConverter
> --
>
> Key: CALCITE-2997
> URL: https://issues.apache.org/jira/browse/CALCITE-2997
> Project: Calcite
>  Issue Type: Bug
>Reporter: Jin Xing
>Assignee: Julian Hyde
>Priority: Major
>
> In current code, *SqlToRelConverter:createJoin* is calling 
> *RelOptUtil.pushDownJoinConditions* for optimization. And we can find below 
> conversion from *SqlNode* to *RelNode*:
> {code:java}
> SqlNode:
> select * from A join B on A.x = B.x * 2
> RelNode (Logical-Plan):
> Join (condition:col0=col1)
> |-Project(x as col0)
> | |-Scan(A)
> |-Project(x * 2 as col1)
>   |-Scan(B){code}
> As we can see the logical plan(*RelNode*) posted above is not the pure 
> reflection of the original SQL String(*SqlNode*). The optimization is mixed 
> into the phase on which AST is converted to Logical-Plan. Actually optimizing 
> rule of JoinPushExpressionsRule is doing exactly the same kind of thing. 
> Shall we just keep the optimization inside Optimized-Logical-Plan ? I mean 
> shall we avoid calling *RelOptUtil.pushDownJoinConditions* in 
> *SqlToRelConverter:createJoin*
> I raised this issue because that we are doing something based on the 
> Logical-Plan. And it makes us really confused that the Logical-Plan doesn't 
> corresponds to SqlNode. 
>  
>  



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