[
https://issues.apache.org/jira/browse/CALCITE-4913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17545762#comment-17545762
]
Stamatis Zampetakis commented on CALCITE-4913:
----------------------------------------------
Hey [~korlov] , thanks for pushing this forward. I would really like to
finalize this work and get it in the next release so I will try to do my best.
>From your recent
>[email|https://lists.apache.org/thread/p14pjfo003rh3vo9fz1zrtrl2zpxktww] to
>the dev list and after having another look in the PR it seems that you have
>found additional bugs in Calcite. Since the current PR is becoming relatively
>big I would like to see if we can somehow split it up into pieces (if it makes
>sense) and delegate some work to follow up JIRAs/PRs. Smaller PRs are easier
>to get in and also the history is easier to follow from future maintainers.
I get the impression we can divide the PR into 4-5 parts:
# correlate variable deduplication in the SELECT clause when going from SQL to
plan
# expose the correlation variables used in Project
# wrong data type in correlate variables after field trimming
# new RelBuilder APIs to create project with correlations
# update rules etc to propagate correlate variables in project
The main problem in this JIRA seems to be the conversion from SQL to RelNode
(Point 1) so let's try to focus on this here and treat the rest in follow-up
JIRAs/PRs.
I don't remember of why we need to expose the correlation variables in the
Project in order to solve the problem so can't really say for sure if this can
go in another JIRA or not. I think this was the main reason of why we had to
touch a lot of places in the code so if we can avoid this for now for sure we
can merge the first one much faster.
It is good that you addressed also the field trimming problem but if it is not
strictly related with the main problem reported here I would create another
JIRA and have a separate discussion there.
I am pretty confident that we can leave the changes around the RelBuilder for
another JIRA since the new APIs are not used by production code. In my
experience changes to RelBuilder take quite a few iterations till they get in
so it seems better to leave them out for now.
The need for updating the rules came from the fact that we added a new field in
the Project holding explicitly the correlated variables that also came from the
attempt to expose the variables. If we don't necessary need to expose the
variables here then possibly we can also address this propagation in a follow
up. I am also thinking that even if we want to expose the variables it may be
possible to do it without introducing a new constructor but rather extract them
from the RexNode expressions. Again if we can postpone this discussion for
another JIRA it could speed up things.
[~korlov] I would like to know what you think about the above. If you agree
that splitting the PR makes sense then you can close
[PR#2623|https://github.com/apache/calcite/pull/2623] rebase and squash as you
see fit and raise a new PR for this issue focusing on Point 1 and I will review
ASAP. Then you can create follow-up JIRAs/PRs as needed. On the other hand, if
you think that it is not possible to divide PR#2623 further then we can
continue iterating on this one.
> 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: 11h 40m
> 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)
