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

Julian Hyde commented on CALCITE-4668:
--------------------------------------

[~jamesstarr], This seems like a useful change. But RelBuilder now scans the 
entire tree of the RHS of a join. That is potentially a Cartesian operation. 
Are you sure there are no performance issues in this change?

Can you improve the doc of isCorrelated describing what it returns? (When a 
method says “Checks...” I assume it’s going to throw.)

Do you still require that there is precisely one correlation variable?

Add description to your test methods, explaining what they return and why. The 
reasoning is too subtle for a maintainer to understand. 

In your subject and commit message, change “exists” to “is used”. 

Several sub-query.iq tests are now trivial. That’s good news, I guess. But 
maybe it means that what used to be tested is no longer tested. So maybe you 
need to change those test queries or write new ones.  

[~zabetak], I saw your review on the PR. Please don’t merge until the issues I 
raised above have been resolved?

> RelBuilder.join should only emit a correlate when a correlate variable exists
> -----------------------------------------------------------------------------
>
>                 Key: CALCITE-4668
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4668
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: James Starr
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> We generate correlated join when no correlate variable is present on the 
> right side.  However, this could be simplified to emit a normal join.
> Given:
> {code:java}
> final RelNode rel = builder
>     .scan("EMP")
>     .variable(v)
>     .scan("DEPT")
>     .join(JoinRelType.LEFT,
>         builder.equals(builder.field(2, 0, "SAL"),
>             builder.literal(1000)),
>         ImmutableSet.of(v.get().id))
>     .build();
> {code}
> Currently the following is emitted:
> {code}
> LogicalFilter(condition=[=($7, $8)])
>   LogicalCorrelate(correlation=[$cor0], joinType=[left], 
> requiredColumns=[{0}])
>     LogicalTableScan(table=[[scott, EMP]])
>     LogicalFilter(condition=[=($cor0.DEPTNO, $0)])
>       LogicalTableScan(table=[[scott, DEPT]])
> {code}
> After the changes the following will be emitted:
> {code}
> LogicalJoin(condition=[=($7, $8)], joinType=[left])
>   LogicalTableScan(table=[[scott, EMP]])
>   LogicalTableScan(table=[[scott, DEPT]])
> {code}



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

Reply via email to