[ 
https://issues.apache.org/jira/browse/CALCITE-6261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17817455#comment-17817455
 ] 

Niels Pardon commented on CALCITE-6261:
---------------------------------------

[~pmsgd] I would use the following unit test to recreate your reproducer from 
CALCITE-5888, please tell me if you think this correctly represents your issue. 
It uses slightly less verbose syntax and the existing tables used in 
RelBuilderTest instead of your CalciqueTable classes. Though it should be 
structurally the same.
{code:java}
@Test void 
testAggregateDuplicateAggCallsAndFieldPruningWithJoinAndLiteralGroupKey() {
  final Function<RelBuilder, RelNode> f1 = builder ->
      // first inner join two tables
      builder.scan("EMP").scan("DEPT")
          .join(JoinRelType.INNER, "DEPTNO")
          .aggregate(
              // null group key
              builder.groupKey(builder.cast(builder.literal(null), 
SqlTypeName.INTEGER)),
              // duplicated min/max agg calls
              builder.min(builder.field("SAL")),
              builder.max(builder.field("SAL")),
              builder.min(builder.field("SAL")),
              builder.max(builder.field("SAL")))
          .build();
  final String expected = ""
      + "LogicalProject($f11=[$0], $f1=[$1], $f2=[$2], $f10=[$1], $f20=[$2])\n"
      + "  LogicalAggregate(group=[{1}], agg#0=[MIN($0)], agg#1=[MAX($0)])\n"
      + "    LogicalProject(SAL=[$5], $f11=[null:INTEGER])\n"
      + "      LogicalJoin(condition=[=($7, $8)], joinType=[inner])\n"
      + "        LogicalTableScan(table=[[scott, EMP]])\n"
      + "        LogicalTableScan(table=[[scott, DEPT]])\n";
  assertThat(f1.apply(createBuilder()), hasTree(expected));
} {code}

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

Reply via email to