[
https://issues.apache.org/jira/browse/CALCITE-6261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17819501#comment-17819501
]
Phani Mahidhar commented on CALCITE-6261:
-----------------------------------------
[~zrlpar] my apologies, was excited to make my first contribution and realised
a small and quick fix so raised the PR. please go ahead with your PR if you
feel my fix is incomplete or can be improved.
> RelBuilder.aggregate() field pruning does not use permuted field indices when
> used with force project and duplicate agg calls
> -----------------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-6261
> URL: https://issues.apache.org/jira/browse/CALCITE-6261
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.35.0, 1.36.0
> Reporter: Niels Pardon
> Assignee: Niels Pardon
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.37.0
>
>
> We were running into issues with RelBuilder.aggregate() when we tried to
> upgrade our code from Calcite 1.32.0 to 1.36.0 and after some debugging it
> seems this is caused by changes introduced with CALCITE-4334.
> I was able to put together this test case for RelBuilderTest reproducing the
> issue:
> {code:java}
> @Test void testAggregateForceProject() {
> final Function<RelBuilder, RelBuilder> f1 = builder ->
> builder.scan("EMP")
> .project(
> ImmutableList.of(
> builder.field("EMPNO"),
> builder.field("ENAME"),
> builder.field("JOB"),
> builder.field("MGR"),
> builder.field("HIREDATE"),
> builder.field("SAL"),
> builder.field("COMM"),
> builder.field("DEPTNO")),
> ImmutableList.of(),
> true);
> final Function<RelBuilder, RelBuilder> f2 = builder ->
> builder.aggregate(
> builder.groupKey(builder.field("MGR")),
> builder.avg(false, "SALARY_AVG", builder.field("SAL")),
> builder.sum(false, "SALARY_SUM", builder.field("SAL")),
> builder.avg(false, "SALARY_MEAN", builder.field("SAL")));
> final String expected = ""
> + "LogicalProject(MGR=[$0], SALARY_AVG=[$1], SALARY_SUM=[$2],
> SALARY_MEAN=[$1])\n"
> + " LogicalAggregate(group=[{0}], SALARY_AVG=[AVG($1)],
> SALARY_SUM=[SUM($1)])\n"
> + " LogicalProject(MGR=[$3], SAL=[$5])\n"
> + " LogicalTableScan(table=[[scott, EMP]])\n";
> assertThat(f2.apply(f1.apply(createBuilder())).build(), hasTree(expected));
> } {code}
> With Calcite 1.36.0 you will get an AssertionError:
> {code:java}
> java.lang.AssertionError
> at org.apache.calcite.rel.core.Aggregate.<init>(Aggregate.java:175)
> at
> org.apache.calcite.rel.logical.LogicalAggregate.<init>(LogicalAggregate.java:72)
> at
> org.apache.calcite.rel.logical.LogicalAggregate.create_(LogicalAggregate.java:144)
> at
> org.apache.calcite.rel.logical.LogicalAggregate.create(LogicalAggregate.java:116)
> at
> org.apache.calcite.rel.core.RelFactories$AggregateFactoryImpl.createAggregate(RelFactories.java:333)
> at org.apache.calcite.tools.RelBuilder.aggregate_(RelBuilder.java:2576)
> at org.apache.calcite.tools.RelBuilder.aggregate_(RelBuilder.java:2523)
> at org.apache.calcite.tools.RelBuilder.aggregate(RelBuilder.java:2335)
> at
> org.apache.calcite.test.RelBuilderTest.lambda$testAggregateForceProject$36(RelBuilderTest.java:1509)
> at
> org.apache.calcite.test.RelBuilderTest.testAggregateForceProject(RelBuilderTest.java:1522)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
> at
> org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
> 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.SameThreadTimeoutInvocation.proceed(SameThreadTimeoutInvocation.java:45)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
> at
> org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
> at
> org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
> 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.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
> at
> org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
> 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
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:129)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
> 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
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:129)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
> 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.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
> at
> java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
> at
> java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
> at
> java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
> at
> java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
> at
> java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)
> {code}
> The issue is caused by the introduction of groupSet2 and groupSets2 which now
> contain the permuted field indices while groupSet/groupSets still uses the
> unchanged field indices and the last portion of the aggregate_() method in
> RelBuilder still using the original groupSet and groupSets variables instead
> of the newly introduced groupSet2 / groupSets2:
> https://github.com/apache/calcite/blob/abf462a7ddab11a44610827b0cd69510c099d9b6/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L2509-L2528
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
