Return-Path: X-Original-To: apmail-calcite-commits-archive@www.apache.org Delivered-To: apmail-calcite-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 782D8185A5 for ; Sun, 13 Dec 2015 22:01:28 +0000 (UTC) Received: (qmail 83601 invoked by uid 500); 13 Dec 2015 22:01:28 -0000 Delivered-To: apmail-calcite-commits-archive@calcite.apache.org Received: (qmail 83535 invoked by uid 500); 13 Dec 2015 22:01:28 -0000 Mailing-List: contact commits-help@calcite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@calcite.apache.org Delivered-To: mailing list commits@calcite.apache.org Received: (qmail 83378 invoked by uid 99); 13 Dec 2015 22:01:28 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 13 Dec 2015 22:01:28 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0B5ADE0454; Sun, 13 Dec 2015 22:01:28 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jhyde@apache.org To: commits@calcite.apache.org Date: Sun, 13 Dec 2015 22:01:33 -0000 Message-Id: <0b02d65869894609af36d219dc015cb9@git.apache.org> In-Reply-To: <3802dd44e90d4ac9bf3b3dc7ac9f4bde@git.apache.org> References: <3802dd44e90d4ac9bf3b3dc7ac9f4bde@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [7/7] calcite git commit: [CALCITE-1024] In a planner test, if a rule should have no effect, state that explicitly [CALCITE-1024] In a planner test, if a rule should have no effect, state that explicitly Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/d55ff83f Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/d55ff83f Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/d55ff83f Branch: refs/heads/master Commit: d55ff83ff3a45b6adffb840daefac3cd2aea34e5 Parents: 02e7ad6 Author: Julian Hyde Authored: Fri Dec 11 16:30:19 2015 -0800 Committer: Julian Hyde Committed: Sat Dec 12 13:41:15 2015 -0800 ---------------------------------------------------------------------- .../apache/calcite/test/RelOptRulesTest.java | 86 +++++++------ .../org/apache/calcite/test/RelOptTestBase.java | 45 ++++++- .../org/apache/calcite/test/RelOptRulesTest.xml | 123 +++---------------- 3 files changed, 100 insertions(+), 154 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/d55ff83f/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index 4c72ba5..1be8331 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -229,12 +229,11 @@ public class RelOptRulesTest extends RelOptTestBase { HepProgram.builder() .addRuleInstance(FilterJoinRule.FILTER_ON_JOIN) .build(); + final String sql = "select *\n" + + "from dept left join emp using (deptno)\n" + + "where emp.deptno is not null and emp.sal > 100"; checkPlanning(tester.withDecorrelation(true).withTrim(true), preProgram, - new HepPlanner(program), - "select * from dept where exists (\n" - + " select * from emp\n" - + " where emp.deptno = dept.deptno\n" - + " and emp.sal > 100)"); + new HepPlanner(program), sql); } @Test public void testFullOuterJoinSimplificationToLeftOuter() { @@ -280,7 +279,8 @@ public class RelOptRulesTest extends RelOptTestBase { + " where dname = 'Charlie'"); } - private void basePushFilterPastAggWithGroupingSets() throws Exception { + private void basePushFilterPastAggWithGroupingSets(boolean unchanged) + throws Exception { final HepProgram preProgram = HepProgram.builder() .addRuleInstance(ProjectMergeRule.INSTANCE) @@ -290,15 +290,16 @@ public class RelOptRulesTest extends RelOptTestBase { HepProgram.builder() .addRuleInstance(FilterAggregateTransposeRule.INSTANCE) .build(); - checkPlanning(tester, preProgram, new HepPlanner(program), "${sql}"); + checkPlanning(tester, preProgram, new HepPlanner(program), "${sql}", + unchanged); } @Test public void testPushFilterPastAggWithGroupingSets1() throws Exception { - basePushFilterPastAggWithGroupingSets(); + basePushFilterPastAggWithGroupingSets(true); } @Test public void testPushFilterPastAggWithGroupingSets2() throws Exception { - basePushFilterPastAggWithGroupingSets(); + basePushFilterPastAggWithGroupingSets(false); } /** Test case for @@ -316,8 +317,13 @@ public class RelOptRulesTest extends RelOptTestBase { * [CALCITE-799] * Incorrect result for {@code HAVING count(*) > 1}. */ @Test public void testPushFilterPastAggThree() { - checkPlanning(FilterAggregateTransposeRule.INSTANCE, - "select deptno from emp group by deptno having count(*) > 1"); + final HepProgram program = + HepProgram.builder() + .addRuleInstance(FilterAggregateTransposeRule.INSTANCE) + .build(); + final String sql = "select deptno from emp\n" + + "group by deptno having count(*) > 1"; + checkPlanUnchanged(new HepPlanner(program), sql); } /** Test case for @@ -473,18 +479,18 @@ public class RelOptRulesTest extends RelOptTestBase { * ReduceExpressionsRule tries to reduce SemiJoin condition to non-equi * condition. */ @Test public void testSemiJoinReduceConstants() { - final HepProgram preProgram = HepProgram.builder().addRuleInstance( - SemiJoinRule.INSTANCE) - .build(); - - final HepProgram program = HepProgram.builder().addRuleInstance( - ReduceExpressionsRule.JOIN_INSTANCE) - .build(); + final HepProgram preProgram = HepProgram.builder() + .addRuleInstance(SemiJoinRule.INSTANCE) + .build(); + final HepProgram program = HepProgram.builder() + .addRuleInstance(ReduceExpressionsRule.JOIN_INSTANCE) + .build(); + final String sql = "select e1.sal\n" + + " from (select * from emp where deptno = 200) as e1\n" + + "where e1.deptno in (\n" + + " select e2.deptno from emp e2 where e2.sal = 100)"; checkPlanning(tester.withDecorrelation(false).withTrim(true), preProgram, - new HepPlanner(program), - "select e1.sal from (select * from emp where deptno = 200) as e1\n" - + "where e1.deptno in (\n" - + " select e2.deptno from emp e2 where e2.sal = 100)"); + new HepPlanner(program), sql, true); } protected void semiJoinTrim() { @@ -845,10 +851,10 @@ public class RelOptRulesTest extends RelOptTestBase { .addRuleInstance(ReduceExpressionsRule.JOIN_INSTANCE) .build(); - checkPlanning(program, - "select d.deptno" - + " from dept d" - + " where d.deptno=7 and d.deptno=8"); + final String sql = "select d.deptno" + + " from dept d" + + " where d.deptno=7 and d.deptno=8"; + checkPlanUnchanged(new HepPlanner(program), sql); } /** Test case for @@ -980,8 +986,9 @@ public class RelOptRulesTest extends RelOptTestBase { .setExecutor(null); // Rule should not fire, but there should be no NPE - checkPlanning(program, - "select * from (values (1,2)) where 1 + 2 > 3 + CAST(NULL AS INTEGER)"); + final String sql = + "select * from (values (1,2)) where 1 + 2 > 3 + CAST(NULL AS INTEGER)"; + checkPlanUnchanged(new HepPlanner(program), sql); } @Test public void testAlreadyFalseEliminatesFilter() throws Exception { @@ -1754,26 +1761,27 @@ public class RelOptRulesTest extends RelOptTestBase { } @Test public void testPushFilterWithRank() throws Exception { - HepProgram program = new HepProgramBuilder().addRuleInstance( - FilterProjectTransposeRule.INSTANCE).build(); + HepProgram program = new HepProgramBuilder() + .addRuleInstance(FilterProjectTransposeRule.INSTANCE).build(); final String sql = "select e1.ename, r\n" + "from (\n" + " select ename, " + " rank() over(partition by deptno order by sal) as r " + " from emp) e1\n" + "where r < 2"; - checkPlanning(program, sql); + checkPlanUnchanged(new HepPlanner(program), sql); } @Test public void testPushFilterWithRankExpr() throws Exception { - HepProgram program = new HepProgramBuilder().addRuleInstance( - FilterProjectTransposeRule.INSTANCE).build(); - checkPlanning(program, "select e1.ename, r\n" + HepProgram program = new HepProgramBuilder() + .addRuleInstance(FilterProjectTransposeRule.INSTANCE).build(); + final String sql = "select e1.ename, r\n" + "from (\n" + " select ename,\n" + " rank() over(partition by deptno order by sal) + 1 as r " + " from emp) e1\n" - + "where r < 2"); + + "where r < 2"; + checkPlanUnchanged(new HepPlanner(program), sql); } /** Test case for @@ -1853,7 +1861,7 @@ public class RelOptRulesTest extends RelOptTestBase { + "from (select * from sales.emp where empno = 10) as e " + "join sales.dept as d on e.empno < d.deptno " + "group by e.empno,d.deptno"; - checkPlanning(tester, preProgram, new HepPlanner(program), sql); + checkPlanning(tester, preProgram, new HepPlanner(program), sql, true); } /** SUM is the easiest aggregate function to split. */ @@ -1977,9 +1985,9 @@ public class RelOptRulesTest extends RelOptTestBase { .build(); // This one cannot be pushed down final String sql = "select * from sales.emp left join (\n" - + "select * from sales.dept) using (deptno)\n" - + "order by sal, name limit 10"; - checkPlanning(tester, preProgram, new HepPlanner(program), sql); + + "select * from sales.dept) using (deptno)\n" + + "order by sal, name limit 10"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql, true); } /** Test case for http://git-wip-us.apache.org/repos/asf/calcite/blob/d55ff83f/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java b/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java index 777991a..c6db96c 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptTestBase.java @@ -32,6 +32,8 @@ import com.google.common.collect.Lists; import java.util.List; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** @@ -85,6 +87,17 @@ abstract class RelOptTestBase extends SqlToRelTestBase { } /** + * Checks that the plan is the same before and after executing a given + * planner. Useful for checking circumstances where rules should not fire. + * + * @param planner Planner + * @param sql SQL query + */ + protected void checkPlanUnchanged(RelOptPlanner planner, String sql) { + checkPlanning(tester, null, planner, sql, true); + } + + /** * Checks the plan for a SQL statement before/after executing a given rule, * with a pre-program to prepare the tree. * @@ -93,11 +106,23 @@ abstract class RelOptTestBase extends SqlToRelTestBase { * @param planner Planner * @param sql SQL query */ - protected void checkPlanning( - Tester tester, - HepProgram preProgram, - RelOptPlanner planner, - String sql) { + protected void checkPlanning(Tester tester, HepProgram preProgram, + RelOptPlanner planner, String sql) { + checkPlanning(tester, preProgram, planner, sql, false); + } + + /** + * Checks the plan for a SQL statement before/after executing a given rule, + * with a pre-program to prepare the tree. + * + * @param tester Tester + * @param preProgram Program to execute before comparing before state + * @param planner Planner + * @param sql SQL query + * @param unchanged Whether the rule is to have no effect + */ + protected void checkPlanning(Tester tester, HepProgram preProgram, + RelOptPlanner planner, String sql, boolean unchanged) { final DiffRepository diffRepos = getDiffRepos(); String sql2 = diffRepos.expand("sql", sql); final RelRoot root = tester.convertSqlToRel(sql2); @@ -132,7 +157,15 @@ abstract class RelOptTestBase extends SqlToRelTestBase { RelNode relAfter = planner.findBestExp(); String planAfter = NL + RelOptUtil.toString(relAfter); - diffRepos.assertEquals("planAfter", "${planAfter}", planAfter); + if (unchanged) { + assertThat(planAfter, is(planBefore)); + } else { + diffRepos.assertEquals("planAfter", "${planAfter}", planAfter); + if (planBefore.equals(planAfter)) { + throw new AssertionError("Expected plan before and after is the same.\n" + + "You must use unchanged=true or call checkPlanUnchanged"); + } + } } } http://git-wip-us.apache.org/repos/asf/calcite/blob/d55ff83f/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml index 5c2e3c0..fc82b09 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -216,19 +216,6 @@ LogicalProject(SAL=[$0]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> - - - @@ -417,15 +404,6 @@ LogicalProject(DDEPTNO=[$0], DNAME=[$1], C=[$2]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) ]]> - - - @@ -644,13 +622,6 @@ LogicalProject(DEPTNO=[$0]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) ]]> - - - @@ -2923,41 +2894,27 @@ LogicalProject(EMPNO=[10], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$ - 100)]]> + 100]]> ($5, 100)]) - LogicalTableScan(table=[[CATALOG, SALES, EMP]]) - LogicalAggregate(group=[{0}]) - LogicalProject(DEPTNO=[$0]) - LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +LogicalProject(DEPTNO=[$0], NAME=[$1], EMPNO=[$2], ENAME=[$3], JOB=[$4], MGR=[$5], HIREDATE=[$6], SAL=[$7], COMM=[$8], DEPTNO0=[$9], SLACKER=[$10]) + LogicalFilter(condition=[AND(IS NOT NULL($9), >($7, 100))]) + LogicalJoin(condition=[=($0, $9)], joinType=[left]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> ($5, 100)]) - LogicalTableScan(table=[[CATALOG, SALES, EMP]]) - LogicalAggregate(group=[{0}]) - LogicalProject(DEPTNO=[$0]) - LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +LogicalProject(DEPTNO=[$0], NAME=[$1], EMPNO=[$2], ENAME=[$3], JOB=[$4], MGR=[$5], HIREDATE=[$6], SAL=[$7], COMM=[$8], DEPTNO0=[$9], SLACKER=[$10]) + LogicalProject(DEPTNO=[$0], NAME=[$1], EMPNO=[CAST($2):INTEGER], ENAME=[CAST($3):VARCHAR(20) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary"], JOB=[CAST($4):VARCHAR(10) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary"], MGR=[$5], HIREDATE=[CAST($6):TIMESTAMP(0)], SAL=[CAST($7):INTEGER], COMM=[CAST($8):INTEGER], DEPTNO0=[CAST($9):INTEGER], SLACKER=[CAST($10):BOOLEAN]) + LogicalJoin(condition=[=($0, $9)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalFilter(condition=[>($5, 100)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> @@ -3182,14 +3139,6 @@ LogicalProject(ENAME=[$0], R=[$1]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> - - - @@ -3207,14 +3156,6 @@ LogicalProject(ENAME=[$0], R=[$1]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> - - - @@ -3541,13 +3482,6 @@ LogicalProject(EXPR$0=[$0], EXPR$1=[$1]) LogicalValues(tuples=[[{ 1, 2 }]]) ]]> - - (+(1, 2), +(3, null))]) - LogicalValues(tuples=[[{ 1, 2 }]]) -]]> - @@ -3645,16 +3579,6 @@ LogicalAggregate(group=[{0, 9}]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) ]]> - - - @@ -4001,15 +3925,6 @@ LogicalProject(DEPTNO=[$0]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> - - ($1, 1)]) - LogicalAggregate(group=[{0}], agg#0=[COUNT()]) - LogicalProject(DEPTNO=[$7]) - LogicalTableScan(table=[[CATALOG, SALES, EMP]]) -]]> - @@ -4169,16 +4084,6 @@ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$ LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) ]]> - - -