[
https://issues.apache.org/jira/browse/CALCITE-4913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17545906#comment-17545906
]
Stamatis Zampetakis commented on CALCITE-4913:
----------------------------------------------
I went over [PR#2825|https://github.com/apache/calcite/pull/2825] it's much
more contained, thank you for preparing this [~korlov]. Albeit some minor stuff
that I will fix myself I will merge it soon.
I have a few observations/questions which are not blocking for the PR but it
would be great if someone can shed some light around the importance of the
correlation deduplication logic.
The query attached in the PR is the following:
{code:sql}
SELECT deptno,
(SELECT min(1)
FROM emp
WHERE empno > d.deptno) AS i0,
(SELECT min(0)
FROM emp
WHERE deptno = d.deptno
AND ename = 'SMITH'
AND d.deptno > 0) AS i1
FROM dept AS d
{code}
The part of the plan that changes with and without the changes is shown below:
+Subplan before subquery expansion with proposed changes+
{noformat}
LogicalAggregate(group=[{}], EXPR$0=[MIN($0)])
LogicalProject($f0=[0])
LogicalFilter(condition=[AND(=($7, $cor0.DEPTNO), =($1, 'SMITH'),
>($cor0.DEPTNO, 0))])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
{noformat}
+Subplan before subquery expansion without proposed changes+
{noformat}
LogicalAggregate(group=[{}], EXPR$0=[MIN($0)])
LogicalProject($f0=[0])
LogicalFilter(condition=[AND(=($7, $cor1.DEPTNO), =($1, 'SMITH'),
>($cor2.DEPTNO, 0))])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
{noformat}
+Observations:+
* Both plans (with and without the changes) before subquery expansion seem
valid.
* The proposed changes do not seem to affect the plans *after* subquery
expansion.
* The proposed changes do not seem to affect query decorrelation either at
least in this case.
I understand that since the subqueries are referring to the same column it
makes sense to reuse the same correlation variable and this makes the plan
before expansion more intuitive but are there any other benefits of reusing the
same variable? To put the question a bit differently what are the problems if
we don't reuse the correlation variable.
The deduplication logic has been there inside the {{SqlToRelConverter}} for
quite a while so it is very probable that there are pieces of code depending on
this behavior but I didn't find to time experiment with it and see what breaks
if it is removed. It could be an interesting experiment for the future.
> Correlated variables in a select list are not deduplicated
> ----------------------------------------------------------
>
> Key: CALCITE-4913
> URL: https://issues.apache.org/jira/browse/CALCITE-4913
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.28.0
> Reporter: Konstantin Orlov
> Assignee: Konstantin Orlov
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.31.0
>
> Time Spent: 12h 10m
> Remaining Estimate: 0h
>
> Currently, the deduplication of correlated variables ain't done for
> correlates in select list. Looks like the root cause here is same as for
> CALCITE-4673, but, unfortunately, suggested patch has addressed only the
> particular problem with a table function. It would be great to provide a more
> gereric fix.
> Besides, the project rel doesn't expose any correlated variables that should
> be set by this node.
> The problematic query is follow:
> {code:java}
> select e.deptno,
> (select count(*) from emp where e.deptno > 0) as c1,
> (select count(*) from emp where e.deptno > 0 and e.deptno < 10) as c2
> from emp e;
> {code}
> Currently after conversion to relational nodes it looks like:
> {code:java}
> LogicalProject(DEPTNO=[$7], EXPR$1=[$SCALAR_QUERY({
> LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
> LogicalProject($f0=[0])
> LogicalFilter(condition=[>($cor0.DEPTNO, 0)])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> })], EXPR$2=[$SCALAR_QUERY({
> LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
> LogicalProject($f0=[0])
> LogicalFilter(condition=[AND(>($cor1.DEPTNO, 0), <($cor2.DEPTNO, 10))])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> })])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> {code}
> All the correlates are set by the very top project node and could be
> represented by only one variable.
> Moreover, I've tried to add the query above to _sub-query.iq_ and got the
> follow error:
> {code:java}
> > java.sql.SQLException: Error while executing SQL "select e.deptno,
> > (select count(*) from emp where e.deptno > 0) as c1,
> > (select count(*) from emp where e.deptno > 0 and e.deptno < 10) as c2
> > from emp e": cm.mapCorToCorRel.get($cor1)
> > at org.apache.calcite.avatica.Helper.createException(Helper.java:56)
> > at org.apache.calcite.avatica.Helper.createException(Helper.java:41)
> > at
> > org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:163)
> > at
> > org.apache.calcite.avatica.AvaticaStatement.executeQuery(AvaticaStatement.java:227)
> > at net.hydromatic.quidem.Quidem.checkResult(Quidem.java:325)
> > at net.hydromatic.quidem.Quidem.access$2800(Quidem.java:54)
> > at
> > net.hydromatic.quidem.Quidem$ContextImpl.checkResult(Quidem.java:1748)
> > at
> > net.hydromatic.quidem.Quidem$CheckResultCommand.execute(Quidem.java:1079)
> > at
> > net.hydromatic.quidem.Quidem$CompositeCommand.execute(Quidem.java:1549)
> > at net.hydromatic.quidem.Quidem.execute(Quidem.java:216)
> > at org.apache.calcite.test.QuidemTest.checkRun(QuidemTest.java:173)
> > at org.apache.calcite.test.QuidemTest.test(QuidemTest.java:224)
> > 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:725)
> > at
> > org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
> > at
> > org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
> > at
> > org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)
> > at
> > org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
> > at
> > org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
> > at
> > org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestTemplateMethod(TimeoutExtension.java:92)
> > 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:106)
> > at
> > org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
> > at
> > org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
> > at
> > org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
> > 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$7(TestMethodTestDescriptor.java:214)
> > at
> > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> > at
> > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
> > at
> > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
> > at
> > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
> > at
> > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
> > at
> > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> > at
> > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
> > at
> > org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
> > at
> > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
> > at
> > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> > at
> > org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
> > at
> > org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
> > at
> > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:185)
> > 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)
> > Caused by: java.lang.NullPointerException: cm.mapCorToCorRel.get($cor1)
> > at java.util.Objects.requireNonNull(Objects.java:290)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.getCorRel(RelDecorrelator.java:934)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.createValueGenerator(RelDecorrelator.java:826)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateInputWithValueGenerator(RelDecorrelator.java:1034)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.maybeAddValueGenerator(RelDecorrelator.java:953)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:1156)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:1122)
> > at sun.reflect.GeneratedMethodAccessor17.invoke(Unknown Source)
> > at
> > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> > at java.lang.reflect.Method.invoke(Method.java:498)
> > at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:711)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:515)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:498)
> > at sun.reflect.GeneratedMethodAccessor26.invoke(Unknown Source)
> > at
> > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> > at java.lang.reflect.Method.invoke(Method.java:498)
> > at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:711)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:1297)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:1276)
> > 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.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:711)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:753)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:742)
> > at sun.reflect.GeneratedMethodAccessor14.invoke(Unknown Source)
> > at
> > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> > at java.lang.reflect.Method.invoke(Method.java:498)
> > at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:711)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelate(RelDecorrelator.java:306)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateQuery(RelDecorrelator.java:230)
> > at
> > org.apache.calcite.tools.Programs$DecorrelateProgram.run(Programs.java:361)
> > at
> > org.apache.calcite.tools.Programs$SequenceProgram.run(Programs.java:336)
> > at org.apache.calcite.prepare.Prepare.optimize(Prepare.java:177)
> > at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:312)
> > at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:220)
> > at
> > org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(CalcitePrepareImpl.java:647)
> > at
> > org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(CalcitePrepareImpl.java:513)
> > at
> > org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(CalcitePrepareImpl.java:483)
> > at
> > org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(CalciteConnectionImpl.java:249)
> > at
> > org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(CalciteMetaImpl.java:623)
> > at
> > org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:675)
> > at
> > org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:156)
> > ... 47 more
> {code}
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
