[
https://issues.apache.org/jira/browse/CALCITE-4686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17414956#comment-17414956
]
Stamatis Zampetakis commented on CALCITE-4686:
----------------------------------------------
Hi [~jamesstarr], I checked what happens in the {{HepPlanner}} and here is what
I found.
If the rule rewrites only one sub-query per call the remaining sub-queries will
be placed inside a {{Filter}} after (your) transformation. This means that if
you want all the sub-queries of the query to be unnested the rule that you need
is not the {{JOIN_SUB_QUERY_TO_CORRELATE}} rule anymore but the
{{FILTER_SUB_QUERY_TO_CORRELATE}}. With these two rules in place the
{{HepPlanner}} is able to unnest all sub-queries even if
{{SubQueryRemoveRule.matchJoin}} rewrites only one per-call.
There is still a caveat; when you register the rules in the {{HepPlanner}} you
should call {{HepProgram.builder().addRuleCollection(rules)}} and not
{{HepProgram.builder().addRuleInstance(rule)}} since the two APIs are not
equivalent. Effectively, {{RelOptBase#withRule}} uses the latter API which
makes your test fail if the rule does not rewrite everything in one shot.
To summarize, rewriting all sub-queries or only one per rule call does not seem
related to the correctness of the final plan and you can always obtain the same
result.
The filter variant of the {{SubQueryRemoveRule}} is already unnesting all
sub-queries in a call so I think it makes sense to align also the join rule.
This leaves the project variant with a different behavior which we can tackle
in a follow up JIRA.
> SubQueryRemoveRule.matchJoin should correctly rewrite all sub-queries
> ---------------------------------------------------------------------
>
> Key: CALCITE-4686
> URL: https://issues.apache.org/jira/browse/CALCITE-4686
> Project: Calcite
> Issue Type: Improvement
> Reporter: James Starr
> Assignee: James Starr
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.28.0
>
> Time Spent: 3h
> Remaining Estimate: 0h
>
> SubQueryRemoveRule.matchJoin only rewrites the first subquery in an ON
> condition each call. It should rewrite all of them down right side of the
> join in single call. Furthermore, the filter generated is not shifted
> correctly for the scope that it is being applied to.
> RelOptRulesTest.testExpandJoinIn, which currently disabled, throw an
> exception when run because filter is shifted to be applied to in the context
> of the right side but it applied on top of the join. Which can be observed
> by commenting out the litmus check.
> {code:java}
> @Test void testExpandJoinIn() {
> final String sql = "select empno\n"
> + "from sales.emp left join sales.dept\n"
> + "on emp.deptno in (select deptno from sales.emp where empno < 20)";
> checkSubQuery(sql).check();
> }
> {code}
> {noformat}
> RexInputRef index 7 out of range 0..2
> java.lang.AssertionError: RexInputRef index 7 out of range 0..2
> at org.apache.calcite.util.Litmus$1.fail(Litmus.java:32)
> at org.apache.calcite.rex.RexChecker.visitInputRef(RexChecker.java:125)
> at org.apache.calcite.rex.RexChecker.visitInputRef(RexChecker.java:61)
> at org.apache.calcite.rex.RexInputRef.accept(RexInputRef.java:114)
> at org.apache.calcite.rex.RexChecker.visitCall(RexChecker.java:144)
> at org.apache.calcite.rex.RexChecker.visitCall(RexChecker.java:61)
> at org.apache.calcite.rex.RexCall.accept(RexCall.java:189)
> at org.apache.calcite.rel.core.Join.isValid(Join.java:176)
> at
> org.apache.calcite.test.SqlToRelConverterTest$RelValidityChecker.visit(SqlToRelConverterTest.java:4261)
> at org.apache.calcite.rel.BiRel.childrenAccept(BiRel.java:46)
> at org.apache.calcite.rel.RelVisitor.visit(RelVisitor.java:46)
> at
> org.apache.calcite.test.SqlToRelConverterTest$RelValidityChecker.visit(SqlToRelConverterTest.java:4264)
> at org.apache.calcite.rel.SingleRel.childrenAccept(SingleRel.java:72)
> at org.apache.calcite.rel.RelVisitor.visit(RelVisitor.java:46)
> at
> org.apache.calcite.test.SqlToRelConverterTest$RelValidityChecker.visit(SqlToRelConverterTest.java:4264)
> at org.apache.calcite.rel.SingleRel.childrenAccept(SingleRel.java:72)
> at org.apache.calcite.rel.RelVisitor.visit(RelVisitor.java:46)
> at
> org.apache.calcite.test.SqlToRelConverterTest$RelValidityChecker.visit(SqlToRelConverterTest.java:4264)
> at org.apache.calcite.rel.RelVisitor.go(RelVisitor.java:63)
> at
> org.apache.calcite.test.SqlToRelTestBase.assertValid(SqlToRelTestBase.java:154)
> at
> org.apache.calcite.test.RelOptTestBase.checkPlanning(RelOptTestBase.java:163)
> at
> org.apache.calcite.test.RelOptTestBase.checkPlanning(RelOptTestBase.java:109)
> at
> org.apache.calcite.test.RelOptTestBase.access$100(RelOptTestBase.java:66)
> at
> org.apache.calcite.test.RelOptTestBase$Sql.check(RelOptTestBase.java:340)
> at
> org.apache.calcite.test.RelOptTestBase$Sql.check(RelOptTestBase.java:316)
> at
> org.apache.calcite.test.RelOptRulesTest.testExpandJoinIn(RelOptRulesTest.java:5702)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at
> org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
> at
> org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125)
> at
> org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:139)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:131)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:81)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:104)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:62)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:43)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:35)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:202)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:198)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:69)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
> at
> org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:115)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
> at
> org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:115)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
> at
> org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
> at java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
> at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
> at
> java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
> at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
> at
> java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
> {noformat}
> Current plan
> {noformat}
> LogicalProject(EMPNO=[$0])
> LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
> SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], DEPTNO0=[$9], NAME=[$10])
> LogicalJoin(condition=[true], joinType=[left])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalJoin(condition=[=($7, $11)], joinType=[inner])
> LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
> LogicalAggregate(group=[{0}])
> LogicalProject(DEPTNO=[$7])
> LogicalFilter(condition=[<($0, 20)])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> {noformat}
> Correct Plan
> {noformat}
> LogicalProject(EMPNO=[$0])
> LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
> SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], DEPTNO0=[$9], NAME=[$10])
> LogicalCorrelate(correlation=[$cor0], joinType=[left],
> requiredColumns=[{7}])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalJoin(condition=[=($cor0.DEPTNO, $2)], joinType=[inner])
> LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
> LogicalAggregate(group=[{0}])
> LogicalProject(DEPTNO=[$7])
> LogicalFilter(condition=[<($0, 20)])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> {noformat}
> Notice the first Join is changed to a correlate and the second join has a
> correlated variable in the join.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
