You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by hy...@apache.org on 2019/11/12 18:44:59 UTC
[calcite] branch master updated: [CALCITE-3448]
AggregateOnCalcToAggUnifyRule may ignore Project incorrectly (Jin Xing)
This is an automated email from the ASF dual-hosted git repository.
hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new 3853118 [CALCITE-3448] AggregateOnCalcToAggUnifyRule may ignore Project incorrectly (Jin Xing)
3853118 is described below
commit 3853118b4d1d48f7d3970b5488baf7ca78d5028e
Author: jinxing <ji...@gmail.com>
AuthorDate: Fri Oct 25 21:20:04 2019 +0800
[CALCITE-3448] AggregateOnCalcToAggUnifyRule may ignore Project incorrectly (Jin Xing)
CALCITE-3087 fixed AggregateOnProjectToAggregateUnifyRule when mapping under
Aggregate breaks ordering. But it fails when there's missing grouping in query
compared with target.
Below matching fails:
MV:
select empid, deptno, name, count(*) from emps
group by empid, deptno, name
Query:
select name, empid, count(*) from emps group by name, empid
Note that -- comparing groupings in MV (empid, deptno, name) and groupings in
query (name, empid), deptno is missed and ordering is broken
Close #1532
---
.../apache/calcite/plan/SubstitutionVisitor.java | 21 ++++++++++++---------
.../org/apache/calcite/util/mapping/Mappings.java | 8 +++++---
.../apache/calcite/test/MaterializationTest.java | 16 ++++++++++++++++
3 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
index 58f57da..75484b0 100644
--- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
@@ -74,6 +74,7 @@ import org.slf4j.Logger;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -131,7 +132,7 @@ public class SubstitutionVisitor {
JoinOnRightCalcToJoinUnifyRule.INSTANCE,
JoinOnCalcsToJoinUnifyRule.INSTANCE,
AggregateToAggregateUnifyRule.INSTANCE,
- AggregateOnCalcToAggUnifyRule.INSTANCE,
+ AggregateOnCalcToAggregateUnifyRule.INSTANCE,
UnionToUnionUnifyRule.INSTANCE,
UnionOnCalcsToUnionUnifyRule.INSTANCE);
@@ -1411,12 +1412,12 @@ public class SubstitutionVisitor {
* We try to pull up the {@link MutableCalc} to top of {@link MutableAggregate},
* then match the {@link MutableAggregate} in query to {@link MutableAggregate} in target.
*/
- private static class AggregateOnCalcToAggUnifyRule extends AbstractUnifyRule {
+ private static class AggregateOnCalcToAggregateUnifyRule extends AbstractUnifyRule {
- public static final AggregateOnCalcToAggUnifyRule INSTANCE =
- new AggregateOnCalcToAggUnifyRule();
+ public static final AggregateOnCalcToAggregateUnifyRule INSTANCE =
+ new AggregateOnCalcToAggregateUnifyRule();
- private AggregateOnCalcToAggUnifyRule() {
+ private AggregateOnCalcToAggregateUnifyRule() {
super(operand(MutableAggregate.class, operand(MutableCalc.class, query(0))),
operand(MutableAggregate.class, target(0)), 1);
}
@@ -1475,11 +1476,13 @@ public class SubstitutionVisitor {
if (!Mappings.keepsOrdering(mapping)) {
final List<Integer> posList = new ArrayList<>();
final int fieldCount = aggregate2.rowType.getFieldCount();
- for (int group: aggregate2.groupSet) {
- if (inverseMapping.getTargetOpt(group) != -1) {
- posList.add(inverseMapping.getTarget(group));
- }
+ final List<Pair<Integer, Integer>> pairs = new ArrayList<>();
+ final List<Integer> groupings = aggregate2.groupSet.toList();
+ for (int i = 0; i < groupings.size(); i++) {
+ pairs.add(Pair.of(mapping.getTarget(groupings.get(i)), i));
}
+ Collections.sort(pairs);
+ pairs.forEach(pair -> posList.add(pair.right));
for (int i = posList.size(); i < fieldCount; i++) {
posList.add(i);
}
diff --git a/core/src/main/java/org/apache/calcite/util/mapping/Mappings.java b/core/src/main/java/org/apache/calcite/util/mapping/Mappings.java
index 5e16f6d..fe84703 100644
--- a/core/src/main/java/org/apache/calcite/util/mapping/Mappings.java
+++ b/core/src/main/java/org/apache/calcite/util/mapping/Mappings.java
@@ -431,10 +431,12 @@ public abstract class Mappings {
int prevTarget = -1;
for (int i = 0; i < mapping.getSourceCount(); i++) {
int target = mapping.getTargetOpt(i);
- if (target != -1 && target < prevTarget) {
- return false;
+ if (target != -1) {
+ if (target < prevTarget) {
+ return false;
+ }
+ prevTarget = target;
}
- prevTarget = target;
}
return true;
}
diff --git a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
index c7c3c30..1d38d8b 100644
--- a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
+++ b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
@@ -864,6 +864,22 @@ public class MaterializationTest {
+ " EnumerableTableScan(table=[[hr, m0]])"));
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-3448">[CALCITE-3448]
+ * AggregateOnCalcToAggregateUnifyRule ignores Project incorrectly when
+ * there's missing grouping or mapping breaks ordering</a>. */
+ @Test public void testAggregateOnProject5() {
+ checkMaterialize(
+ "select \"empid\", \"deptno\", \"name\", count(*) from \"emps\"\n"
+ + "group by \"empid\", \"deptno\", \"name\"",
+ "select \"name\", \"empid\", count(*) from \"emps\" group by \"name\", \"empid\"",
+ HR_FKUK_MODEL,
+ CalciteAssert.checkResultContains(""
+ + "EnumerableCalc(expr#0..2=[{inputs}], name=[$t1], empid=[$t0], EXPR$2=[$t2])\n"
+ + " EnumerableAggregate(group=[{0, 2}], EXPR$2=[$SUM0($3)])\n"
+ + " EnumerableTableScan(table=[[hr, m0]])"));
+ }
+
@Test public void testAggregateOnProjectAndFilter() {
String mv = ""
+ "select \"deptno\", sum(\"salary\"), count(1)\n"