You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by ch...@apache.org on 2022/05/16 01:57:22 UTC
[calcite] branch main updated: [CALCITE-5081] Group keys of Aggregate are wrongly changed during decorrelation
This is an automated email from the ASF dual-hosted git repository.
chunwei pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 1bce280a29 [CALCITE-5081] Group keys of Aggregate are wrongly changed during decorrelation
1bce280a29 is described below
commit 1bce280a2957326dc5c249cfd079edfd2c54adf4
Author: Benchao Li <li...@gmail.com>
AuthorDate: Sun Apr 24 08:31:30 2022 +0800
[CALCITE-5081] Group keys of Aggregate are wrongly changed during decorrelation
---
.../apache/calcite/sql2rel/RelDecorrelator.java | 11 +--
.../calcite/sql2rel/RelDecorrelatorTest.java | 89 ++++++++++++++++++++++
2 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
index 34f3205cf6..7347eeb64c 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
@@ -532,20 +532,21 @@ public class RelDecorrelator implements ReflectiveVisitor {
int newPos = 0;
- // oldInput has the original group by keys in the front.
+ final List<Integer> groupKeyIndices = rel.getGroupSet().asList();
final NavigableMap<Integer, RexLiteral> omittedConstants = new TreeMap<>();
for (int i = 0; i < oldGroupKeyCount; i++) {
- final RexLiteral constant = projectedLiteral(newInput, i);
+ final int idx = groupKeyIndices.get(i);
+ final RexLiteral constant = projectedLiteral(newInput, idx);
if (constant != null) {
// Exclude constants. Aggregate({true}) occurs because Aggregate({})
// would generate 1 row even when applied to an empty table.
- omittedConstants.put(i, constant);
+ omittedConstants.put(idx, constant);
continue;
}
// add mapping of group keys.
- outputMap.put(i, newPos);
- int newInputPos = requireNonNull(frame.oldToNewOutputs.get(i));
+ outputMap.put(idx, newPos);
+ int newInputPos = requireNonNull(frame.oldToNewOutputs.get(idx));
projects.add(RexInputRef.of2(newInputPos, newInputOutput));
mapNewInputToProjOutputs.put(newInputPos, newPos);
newPos++;
diff --git a/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java b/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java
new file mode 100644
index 0000000000..8a242ba738
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql2rel;
+
+import org.apache.calcite.plan.RelTraitDef;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rex.RexCorrelVariable;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParser;
+import org.apache.calcite.test.CalciteAssert;
+import org.apache.calcite.tools.Frameworks;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.util.Holder;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.apache.calcite.test.Matchers.hasTree;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/**
+ * Tests for {@link RelDecorrelator}.
+ */
+public class RelDecorrelatorTest {
+ public static Frameworks.ConfigBuilder config() {
+ final SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+ return Frameworks.newConfigBuilder()
+ .parserConfig(SqlParser.Config.DEFAULT)
+ .defaultSchema(
+ CalciteAssert.addSchema(rootSchema, CalciteAssert.SchemaSpec.SCOTT_WITH_TEMPORAL))
+ .traitDefs((List<RelTraitDef>) null);
+ }
+
+ @Test void testGroupKeyNotInFrontWhenDecorrelate() {
+ final RelBuilder builder = RelBuilder.create(config().build());
+ final Holder<@Nullable RexCorrelVariable> v = Holder.empty();
+ RelNode before = builder.scan("EMP")
+ .variable(v)
+ .scan("DEPT")
+ .filter(
+ builder.equals(builder.field(0),
+ builder.call(
+ SqlStdOperatorTable.PLUS,
+ builder.literal(10),
+ builder.field(v.get(), "DEPTNO"))))
+ .correlate(JoinRelType.LEFT, v.get().id, builder.field(2, 0, "DEPTNO"))
+ .aggregate(builder.groupKey("ENAME"), builder.max(builder.field("EMPNO")))
+ .build();
+
+ final String planBefore = ""
+ + "LogicalAggregate(group=[{1}], agg#0=[MAX($0)])\n"
+ + " LogicalCorrelate(correlation=[$cor0], joinType=[left], requiredColumns=[{7}])\n"
+ + " LogicalTableScan(table=[[scott, EMP]])\n"
+ + " LogicalFilter(condition=[=($0, +(10, $cor0.DEPTNO))])\n"
+ + " LogicalTableScan(table=[[scott, DEPT]])\n";
+ assertThat(before, hasTree(planBefore));
+
+ RelNode after = RelDecorrelator.decorrelateQuery(before, builder);
+
+ final String planAfter = ""
+ + "LogicalAggregate(group=[{0}], agg#0=[MAX($1)])\n"
+ + " LogicalProject(ENAME=[$1], EMPNO=[$0])\n"
+ + " LogicalJoin(condition=[=($8, $9)], joinType=[left])\n"
+ + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], "
+ + "SAL=[$5], COMM=[$6], DEPTNO=[$7], $f8=[+(10, $7)])\n"
+ + " LogicalTableScan(table=[[scott, EMP]])\n"
+ + " LogicalTableScan(table=[[scott, DEPT]])\n";
+ assertThat(after, hasTree(planAfter));
+ }
+}