[ 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)