[CALCITE-2116] In HepPlanner, ensure that common sub-expressions have the same digest (LeoWangLZ)
Close apache/calcite#597 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/c0f912dd Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/c0f912dd Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/c0f912dd Branch: refs/heads/master Commit: c0f912ddff717eaedb9a2d652e95f232d10e207f Parents: 7b85d44 Author: LeoWangLZ <[email protected]> Authored: Fri Jan 5 10:15:24 2018 +0800 Committer: Julian Hyde <[email protected]> Committed: Tue Jan 9 23:01:29 2018 -0800 ---------------------------------------------------------------------- .../org/apache/calcite/plan/hep/HepPlanner.java | 5 +++- .../org/apache/calcite/test/HepPlannerTest.java | 25 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/c0f912dd/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java index 657460a..d43bbd8 100644 --- a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java +++ b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java @@ -811,6 +811,9 @@ public class HepPlanner extends AbstractRelOptPlanner { rel = rel.copy(rel.getTraitSet(), newInputs); onCopy(oldRel, rel); } + // Compute digest first time we add to DAG, + // otherwise can't get equivVertex for common sub-expression + rel.recomputeDigest(); // try to find equivalent rel only if DAG is allowed if (!noDAG) { @@ -886,7 +889,7 @@ public class HepPlanner extends AbstractRelOptPlanner { if (mapDigestToVertex.get(oldDigest) == vertex) { mapDigestToVertex.remove(oldDigest); } - String newDigest = rel.recomputeDigest(); + String newDigest = rel.getDigest(); // When a transformation happened in one rule apply, support // vertex2 replace vertex1, but the current relNode of // vertex1 and vertex2 is same, http://git-wip-us.apache.org/repos/asf/calcite/blob/c0f912dd/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java b/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java index e3e144b..7a08d0c 100644 --- a/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java +++ b/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java @@ -21,6 +21,7 @@ import org.apache.calcite.plan.hep.HepMatchOrder; 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.core.RelFactories; import org.apache.calcite.rel.logical.LogicalIntersect; import org.apache.calcite.rel.logical.LogicalUnion; @@ -248,6 +249,30 @@ public class HepPlannerTest extends RelOptTestBase { + " (select * from dept) d2"); } + /** Tests that if two relational expressions are equivalent, the planner + * notices, and only applies the rule once. */ + @Test public void testCommonSubExpression() { + // In the following, + // (select 1 from dept where abs(-1)=20) + // occurs twice, but it's a common sub-expression, so the rule should only + // apply once. + HepProgramBuilder programBuilder = HepProgram.builder(); + programBuilder.addRuleInstance(FilterToCalcRule.INSTANCE); + + final HepTestListener listener = new HepTestListener(0); + HepPlanner planner = new HepPlanner(programBuilder.build()); + planner.addListener(listener); + + final String sql = "(select 1 from dept where abs(-1)=20)\n" + + "union all\n" + + "(select 1 from dept where abs(-1)=20)"; + planner.setRoot(tester.convertSqlToRel(sql).rel); + RelNode bestRel = planner.findBestExp(); + + assertThat(bestRel.getInput(0).equals(bestRel.getInput(1)), is(true)); + assertThat(listener.getApplyTimes() == 1, is(true)); + } + @Test public void testSubprogram() throws Exception { // Verify that subprogram gets re-executed until fixpoint. // In this case, the first time through we limit it to generate
