From ce1d2f7f07ee49671d037df21237dca612c51c06 Mon Sep 17 00:00:00 2001 From: xurenhe Date: Tue, 8 Feb 2022 16:56:55 +0800 Subject: [PATCH 1/4] [CALCITE-5002] SubstitutionVisitor exec mv-match fail, cause by `unused project on aggregate in the mv` --- .../calcite/plan/SubstitutionVisitor.java | 16 ++++++++++++++-- ...terializedViewSubstitutionVisitorTest.java | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 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 8044fca68957..e03cfe98a7ae 100644 --- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java +++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java @@ -195,8 +195,20 @@ public SubstitutionVisitor(RelNode target_, RelNode query_, this.simplify = new RexSimplify(cluster.getRexBuilder(), predicates, executor); this.rules = rules; - this.query = Holder.of(MutableRels.toMutable(query_)); - this.target = MutableRels.toMutable(target_); + MutableRel queryMutableRel = MutableRels.toMutable(query_); + final MutableRel targetMutableRel = MutableRels.toMutable(target_); + if (!(queryMutableRel instanceof MutableCalc) && targetMutableRel instanceof MutableCalc) { + final RelDataType rowType = queryMutableRel.rowType; + final RexProgramBuilder programBuilder = + new RexProgramBuilder(rowType, cluster.getRexBuilder()); + final List queryFields = rowType.getFieldList(); + for (int i = 0; i < queryFields.size(); i++) { + programBuilder.addProject(RexInputRef.of(i, rowType), queryFields.get(i).getName()); + } + queryMutableRel = MutableCalc.of(queryMutableRel, programBuilder.getProgram()); + } + this.query = Holder.of(queryMutableRel); + this.target = targetMutableRel; this.relBuilder = relBuilderFactory.create(cluster, null); final Set<@Nullable MutableRel> parents = Sets.newIdentityHashSet(); final List allNodes = new ArrayList<>(); diff --git a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java index edabf76d4f56..2f192ce775c4 100644 --- a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java +++ b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java @@ -449,6 +449,25 @@ protected final MaterializedViewFixture sql(String materialize, sql(mv, query).noMat(); } + /** + * It's need be matched, unused project on aggregate in the mv, and query's rel-root is aggregate. + */ + @Test void testAggregate7() { + String mv = "" + + "select \"deptno\", sum(\"salary\"), sum(\"commission\") + 1, sum(\"k\")\n" + + "from\n" + + " (select \"deptno\", \"salary\", \"commission\", 100 as \"k\"\n" + + " from \"emps\")\n" + + "group by \"deptno\""; + String query = "" + + "select \"deptno\", sum(\"salary\"), sum(\"k\")\n" + + "from\n" + + " (select \"deptno\", \"salary\", 100 as \"k\"\n" + + " from \"emps\")\n" + + "group by \"deptno\""; + sql(mv, query).ok(); + } + /** * There will be a compensating Project added after matching of the Aggregate. * This rule targets to test if the Calc can be handled. From 1ee2a1d9d954ffe1aefc284c80c714d7b6703bda Mon Sep 17 00:00:00 2001 From: xurenhe Date: Tue, 15 Feb 2022 14:27:44 +0800 Subject: [PATCH 2/4] Add some test for other rels, such as union --- ...terializedViewSubstitutionVisitorTest.java | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java index 2f192ce775c4..29cfc4444706 100644 --- a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java +++ b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java @@ -450,9 +450,10 @@ protected final MaterializedViewFixture sql(String materialize, } /** - * It's need be matched, unused project on aggregate in the mv, and query's rel-root is aggregate. + * Need matching because query could be expressed by mv, using trim mv's unused field, + * which is the top of Aggregate. */ - @Test void testAggregate7() { + @Test void testAggregateWithCalcTopInMv() { String mv = "" + "select \"deptno\", sum(\"salary\"), sum(\"commission\") + 1, sum(\"k\")\n" + "from\n" @@ -468,6 +469,28 @@ protected final MaterializedViewFixture sql(String materialize, sql(mv, query).ok(); } + /** + * Need matching because query could be expressed by mv, using trim mv's unused field, + * which is the top of Union. + */ + @Test void testUnionWithCalcTopInMv() { + String mv = "" + + "select \"deptno\", \"salary\", 'hello' as \"k\"\n" + + "from (" + + "select \"deptno\", \"salary\"\n" + + "from \"emps\"\n" + + "union\n" + + "select \"deptno\", \"salary\"\n" + + "from \"emps\")"; + String query = "" + + "select \"deptno\", \"salary\"\n" + + "from \"emps\"\n" + + "union\n" + + "select \"deptno\", \"salary\"\n" + + "from \"emps\""; + sql(mv, query).ok(); + } + /** * There will be a compensating Project added after matching of the Aggregate. * This rule targets to test if the Calc can be handled. @@ -880,11 +903,14 @@ protected final MaterializedViewFixture sql(String materialize, String m = "select * from \"emps\" where \"empid\" < 500"; sql(m, q) .checkingThatResultContains("" - + "LogicalUnion(all=[true])\n" - + " LogicalCalc(expr#0..4=[{inputs}], expr#5=[300], expr#6=[>($t0, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" - + " LogicalTableScan(table=[[hr, emps]])\n" - + " LogicalCalc(expr#0..4=[{inputs}], expr#5=[200], expr#6=[<($t0, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[hr, MV0]])") + + "LogicalCalc(expr#0..4=[{inputs}], proj#0..4=[{exprs}])\n" + + " LogicalUnion(all=[true])\n" + + " LogicalCalc(expr#0..4=[{inputs}], expr#5=[300], expr#6=[>($t0, $t5)], proj#0." + + ".4=[{exprs}], $condition=[$t6])\n" + + " LogicalTableScan(table=[[hr, emps]])\n" + + " LogicalCalc(expr#0..4=[{inputs}], expr#5=[200], expr#6=[<($t0, $t5)], proj#0." + + ".4=[{exprs}], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[hr, MV0]])") .ok(); } From 34500d20d224bb4f8f668081d77413aff7cf5543 Mon Sep 17 00:00:00 2001 From: xurenhe Date: Tue, 15 Feb 2022 15:41:17 +0800 Subject: [PATCH 3/4] change the way with pre-normalizating rel of target and query. --- .../calcite/plan/SubstitutionVisitor.java | 16 +------- ...terializedViewSubstitutionVisitorTest.java | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 16 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 e03cfe98a7ae..8044fca68957 100644 --- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java +++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java @@ -195,20 +195,8 @@ public SubstitutionVisitor(RelNode target_, RelNode query_, this.simplify = new RexSimplify(cluster.getRexBuilder(), predicates, executor); this.rules = rules; - MutableRel queryMutableRel = MutableRels.toMutable(query_); - final MutableRel targetMutableRel = MutableRels.toMutable(target_); - if (!(queryMutableRel instanceof MutableCalc) && targetMutableRel instanceof MutableCalc) { - final RelDataType rowType = queryMutableRel.rowType; - final RexProgramBuilder programBuilder = - new RexProgramBuilder(rowType, cluster.getRexBuilder()); - final List queryFields = rowType.getFieldList(); - for (int i = 0; i < queryFields.size(); i++) { - programBuilder.addProject(RexInputRef.of(i, rowType), queryFields.get(i).getName()); - } - queryMutableRel = MutableCalc.of(queryMutableRel, programBuilder.getProgram()); - } - this.query = Holder.of(queryMutableRel); - this.target = targetMutableRel; + this.query = Holder.of(MutableRels.toMutable(query_)); + this.target = MutableRels.toMutable(target_); this.relBuilder = relBuilderFactory.create(cluster, null); final Set<@Nullable MutableRel> parents = Sets.newIdentityHashSet(); final List allNodes = new ArrayList<>(); diff --git a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java index 29cfc4444706..70eebeffa8b1 100644 --- a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java +++ b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java @@ -19,18 +19,22 @@ import org.apache.calcite.jdbc.JavaTypeFactoryImpl; import org.apache.calcite.plan.RelOptMaterialization; import org.apache.calcite.plan.RelOptPredicateList; +import org.apache.calcite.plan.RelOptUtil; import org.apache.calcite.plan.SubstitutionVisitor; import org.apache.calcite.plan.hep.HepPlanner; import org.apache.calcite.plan.hep.HepProgram; import org.apache.calcite.plan.hep.HepProgramBuilder; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.logical.LogicalCalc; import org.apache.calcite.rel.rules.CoreRules; import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeField; import org.apache.calcite.rel.type.RelDataTypeSystem; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexInputRef; import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexProgramBuilder; import org.apache.calcite.rex.RexSimplify; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.sql.fun.SqlStdOperatorTable; @@ -80,9 +84,20 @@ public class MaterializedViewSubstitutionVisitorTest { @Override protected List optimize(RelNode queryRel, List materializationList) { RelOptMaterialization materialization = materializationList.get(0); + RelNode materializedRel = canonicalize(materialization.queryRel); + RelNode normalQueryRel = canonicalize(queryRel); + if (!(normalQueryRel instanceof LogicalCalc) && materializedRel instanceof LogicalCalc) { + final RelDataType rowType = normalQueryRel.getRowType(); + final RexProgramBuilder programBuilder = + new RexProgramBuilder(rowType, normalQueryRel.getCluster().getRexBuilder()); + final List queryFields = rowType.getFieldList(); + for (int i = 0; i < queryFields.size(); i++) { + programBuilder.addProject(RexInputRef.of(i, rowType), queryFields.get(i).getName()); + } + normalQueryRel = LogicalCalc.create(normalQueryRel, programBuilder.getProgram()); + } SubstitutionVisitor substitutionVisitor = - new SubstitutionVisitor(canonicalize(materialization.queryRel), - canonicalize(queryRel)); + new SubstitutionVisitor(materializedRel, normalQueryRel); return substitutionVisitor .go(materialization.tableRel); } @@ -469,6 +484,24 @@ protected final MaterializedViewFixture sql(String materialize, sql(mv, query).ok(); } + /** Similar with {@link #testAggregateWithCalcTopInMv()}, + * but target's fields have no-equal sequence. */ + @Test void testAggregateWithCalcTopInMv2() { + String mv = "" + + "select \"deptno\", sum(\"commission\") + 1, sum(\"k\"), sum(\"salary\")\n" + + "from\n" + + " (select \"deptno\", \"salary\", \"commission\", 100 as \"k\"\n" + + " from \"emps\")\n" + + "group by \"deptno\""; + String query = "" + + "select \"deptno\", sum(\"salary\"), sum(\"k\")\n" + + "from\n" + + " (select \"deptno\", \"salary\", 100 as \"k\"\n" + + " from \"emps\")\n" + + "group by \"deptno\""; + sql(mv, query).ok(); + } + /** * Need matching because query could be expressed by mv, using trim mv's unused field, * which is the top of Union. From a497658bad35b10ea432857cff2b240b351f8745 Mon Sep 17 00:00:00 2001 From: xurenhe Date: Tue, 15 Feb 2022 15:50:53 +0800 Subject: [PATCH 4/4] fix format. --- .../calcite/test/MaterializedViewSubstitutionVisitorTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java index 70eebeffa8b1..a5befa40d9f6 100644 --- a/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java +++ b/core/src/test/java/org/apache/calcite/test/MaterializedViewSubstitutionVisitorTest.java @@ -19,7 +19,6 @@ import org.apache.calcite.jdbc.JavaTypeFactoryImpl; import org.apache.calcite.plan.RelOptMaterialization; import org.apache.calcite.plan.RelOptPredicateList; -import org.apache.calcite.plan.RelOptUtil; import org.apache.calcite.plan.SubstitutionVisitor; import org.apache.calcite.plan.hep.HepPlanner; import org.apache.calcite.plan.hep.HepProgram;