[CALCITE-1059] Not valid to convert Aggregate on empty to empty if its GROUP BY key is empty
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/6201013a Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/6201013a Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/6201013a Branch: refs/heads/master Commit: 6201013a9a8fe79c234656ec20c5fe524d68b4d5 Parents: 920a648 Author: Julian Hyde <[email protected]> Authored: Thu Jan 14 17:42:28 2016 -0800 Committer: Julian Hyde <[email protected]> Committed: Thu Jan 21 15:03:41 2016 -0800 ---------------------------------------------------------------------- .../org/apache/calcite/rel/core/Aggregate.java | 7 ++ .../calcite/rel/rules/PruneEmptyRules.java | 78 ++++++++++------- .../apache/calcite/test/RelOptRulesTest.java | 88 +++++++++++++------- .../org/apache/calcite/test/RelOptRulesTest.xml | 34 ++++++++ 4 files changed, 143 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java b/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java index 7b46940..4eedee7 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java @@ -80,6 +80,13 @@ public abstract class Aggregate extends SingleRel { } }; + public static final Predicate<Aggregate> IS_NOT_GRAND_TOTAL = + new Predicate<Aggregate>() { + public boolean apply(Aggregate input) { + return input.getGroupCount() > 0; + } + }; + //~ Instance fields -------------------------------------------------------- public final boolean indicator; http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java index 3fccdfa..8af045a 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java @@ -25,11 +25,17 @@ import org.apache.calcite.rel.core.Aggregate; import org.apache.calcite.rel.core.Filter; import org.apache.calcite.rel.core.Join; import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.core.RelFactories; import org.apache.calcite.rel.core.Sort; import org.apache.calcite.rel.core.Values; import org.apache.calcite.rel.logical.LogicalUnion; import org.apache.calcite.rel.logical.LogicalValues; import org.apache.calcite.rex.RexLiteral; +import org.apache.calcite.tools.RelBuilder; +import org.apache.calcite.tools.RelBuilderFactory; + +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; import java.util.ArrayList; import java.util.List; @@ -71,33 +77,33 @@ public abstract class PruneEmptyRules { "Union") { public void onMatch(RelOptRuleCall call) { LogicalUnion union = call.rel(0); - final List<RelNode> childRels = call.getChildRels(union); - assert childRels != null; - final List<RelNode> newChildRels = new ArrayList<>(); - for (RelNode childRel : childRels) { - if (!isEmpty(childRel)) { - newChildRels.add(childRel); + final List<RelNode> inputs = call.getChildRels(union); + assert inputs != null; + final List<RelNode> newInputs = new ArrayList<>(); + for (RelNode input : inputs) { + if (!isEmpty(input)) { + newInputs.add(input); } } - assert newChildRels.size() < childRels.size() + assert newInputs.size() < inputs.size() : "planner promised us at least one Empty child"; - RelNode newRel; - switch (newChildRels.size()) { + final RelBuilder builder = call.builder(); + switch (newInputs.size()) { case 0: - newRel = empty(union); + builder.push(union).empty(); break; case 1: - newRel = + builder.push( RelOptUtil.createCastRel( - newChildRels.get(0), + newInputs.get(0), union.getRowType(), - true); + true)); break; default: - newRel = LogicalUnion.create(newChildRels, union.all); + builder.push(LogicalUnion.create(newInputs, union.all)); break; } - call.transformTo(newRel); + call.transformTo(builder.build()); } }; @@ -117,7 +123,8 @@ public abstract class PruneEmptyRules { * </ul> */ public static final RelOptRule PROJECT_INSTANCE = - new RemoveEmptySingleRule(Project.class, "PruneEmptyProject"); + new RemoveEmptySingleRule(Project.class, Predicates.<Project>alwaysTrue(), + RelFactories.LOGICAL_BUILDER, "PruneEmptyProject"); /** * Rule that converts a {@link org.apache.calcite.rel.logical.LogicalFilter} @@ -162,7 +169,7 @@ public abstract class PruneEmptyRules { Sort sort = call.rel(0); if (sort.fetch != null && RexLiteral.intValue(sort.fetch) == 0) { - call.transformTo(empty(sort)); + call.transformTo(call.builder().push(sort).empty().build()); } } }; @@ -174,11 +181,15 @@ public abstract class PruneEmptyRules { * <p>Examples: * * <ul> - * <li>Aggregate(Empty) becomes Empty + * <li>{@code Aggregate(key: [1, 3], Empty)} → {@code Empty} + * + * <li>{@code Aggregate(key: [], Empty)} is unchanged, because an aggregate + * without a GROUP BY key always returns 1 row, even over empty input * </ul> */ public static final RelOptRule AGGREGATE_INSTANCE = - new RemoveEmptySingleRule(Aggregate.class, "PruneEmptyAggregate"); + new RemoveEmptySingleRule(Aggregate.class, Aggregate.IS_NOT_GRAND_TOTAL, + RelFactories.LOGICAL_BUILDER, "PruneEmptyAggregate"); /** * Rule that converts a {@link org.apache.calcite.rel.core.Join} @@ -204,7 +215,7 @@ public abstract class PruneEmptyRules { // emp is empty return; } - call.transformTo(empty(join)); + call.transformTo(call.builder().push(join).empty().build()); } }; @@ -232,30 +243,33 @@ public abstract class PruneEmptyRules { // dept is empty return; } - call.transformTo(empty(join)); + call.transformTo(call.builder().push(join).empty().build()); } }; - /** Creates a {@link org.apache.calcite.rel.core.Values} to replace - * {@code node}. */ - private static Values empty(RelNode node) { - return LogicalValues.createEmpty(node.getCluster(), node.getRowType()); - } - /** Planner rule that converts a single-rel (e.g. project, sort, aggregate or * filter) on top of the empty relational expression into empty. */ - private static class RemoveEmptySingleRule extends RelOptRule { - public RemoveEmptySingleRule(Class<? extends SingleRel> clazz, + public static class RemoveEmptySingleRule extends RelOptRule { + /** Creatse a simple RemoveEmptySingleRule. */ + public <R extends SingleRel> RemoveEmptySingleRule(Class<R> clazz, + String description) { + this(clazz, Predicates.<R>alwaysTrue(), RelFactories.LOGICAL_BUILDER, + description); + } + + /** Creatse a RemoveEmptySingleRule. */ + public <R extends SingleRel> RemoveEmptySingleRule(Class<R> clazz, + Predicate<R> predicate, RelBuilderFactory relBuilderFactory, String description) { super( - operand(clazz, + operand(clazz, null, predicate, operand(Values.class, null, Values.IS_EMPTY, none())), - description); + relBuilderFactory, description); } public void onMatch(RelOptRuleCall call) { SingleRel single = call.rel(0); - call.transformTo(empty(single)); + call.transformTo(call.builder().push(single).empty().build()); } } } http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/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 f59b9e7..bfdd32b 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -354,13 +354,11 @@ public class RelOptRulesTest extends RelOptTestBase { .addRuleInstance(filterOnJoin) .addGroupEnd() .build(); - checkPlanning(tester, - preProgram, - new HepPlanner(program), - "select a.name\n" - + "from dept a\n" - + "left join dept b on b.deptno > 10\n" - + "right join dept c on b.deptno > 10\n"); + final String sql = "select a.name\n" + + "from dept a\n" + + "left join dept b on b.deptno > 10\n" + + "right join dept c on b.deptno > 10\n"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql); } @Test public void testJoinProjectTranspose() { @@ -377,13 +375,11 @@ public class RelOptRulesTest extends RelOptTestBase { .addRuleInstance(JoinProjectTransposeRule.LEFT_PROJECT_INCLUDE_OUTER) .addRuleInstance(ProjectMergeRule.INSTANCE) .build(); - checkPlanning(tester, - preProgram, - new HepPlanner(program), - "select a.name\n" - + "from dept a\n" - + "left join dept b on b.deptno > 10\n" - + "right join dept c on b.deptno > 10\n"); + final String sql = "select a.name\n" + + "from dept a\n" + + "left join dept b on b.deptno > 10\n" + + "right join dept c on b.deptno > 10\n"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql); } /** Test case for @@ -1324,6 +1320,36 @@ public class RelOptRulesTest extends RelOptTestBase { "select * from emp order by deptno limit 0"); } + @Test public void testEmptyAggregate() { + HepProgram preProgram = HepProgram.builder() + .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE) + .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE) + .build(); + HepProgram program = new HepProgramBuilder() + .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE) + .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE) + .addRuleInstance(PruneEmptyRules.AGGREGATE_INSTANCE) + .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE) + .build(); + + final String sql = "select sum(empno) from emp where false group by deptno"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql); + } + + @Test public void testEmptyAggregateEmptyKey() { + HepProgram preProgram = HepProgram.builder() + .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE) + .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE) + .build(); + HepProgram program = new HepProgramBuilder() + .addRuleInstance(PruneEmptyRules.AGGREGATE_INSTANCE) + .build(); + + final String sql = "select sum(empno) from emp where false"; + final boolean unchanged = true; + checkPlanning(tester, preProgram, new HepPlanner(program), sql, unchanged); + } + @Test public void testReduceCasts() throws Exception { HepProgram program = new HepProgramBuilder() .addRuleInstance(ReduceExpressionsRule.PROJECT_INSTANCE) @@ -1475,13 +1501,12 @@ public class RelOptRulesTest extends RelOptTestBase { HepProgram program = HepProgram.builder() .addRuleInstance(AggregateFilterTransposeRule.INSTANCE) .build(); - checkPlanning(tester, preProgram, - new HepPlanner(program), - "select empno, sal, deptno from (" - + " select empno, sal, deptno" - + " from emp" - + " where sal > 5000)" - + "group by empno, sal, deptno"); + final String sql = "select empno, sal, deptno from (" + + " select empno, sal, deptno" + + " from emp" + + " where sal > 5000)" + + "group by empno, sal, deptno"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql); } @Test public void testPullFilterThroughAggregateGroupingSets() @@ -1493,13 +1518,12 @@ public class RelOptRulesTest extends RelOptTestBase { HepProgram program = HepProgram.builder() .addRuleInstance(AggregateFilterTransposeRule.INSTANCE) .build(); - checkPlanning(tester, preProgram, - new HepPlanner(program), - "select empno, sal, deptno from (" - + " select empno, sal, deptno" - + " from emp" - + " where sal > 5000)" - + "group by rollup(empno, sal, deptno)"); + final String sql = "select empno, sal, deptno from (" + + " select empno, sal, deptno" + + " from emp" + + " where sal > 5000)" + + "group by rollup(empno, sal, deptno)"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql); } private void basePullConstantTroughAggregate() throws Exception { @@ -1590,15 +1614,15 @@ public class RelOptRulesTest extends RelOptTestBase { public void transitiveInference(RelOptRule... extraRules) throws Exception { final DiffRepository diffRepos = getDiffRepos(); - String sql = diffRepos.expand(null, "${sql}"); + final String sql = diffRepos.expand(null, "${sql}"); - HepProgram program = new HepProgramBuilder() + final HepProgram program = new HepProgramBuilder() .addRuleInstance(FilterJoinRule.DUMB_FILTER_ON_JOIN) .addRuleInstance(FilterJoinRule.JOIN) .addRuleInstance(FilterProjectTransposeRule.INSTANCE) .addRuleInstance(FilterSetOpTransposeRule.INSTANCE) .build(); - HepPlanner planner = new HepPlanner(program); + final HepPlanner planner = new HepPlanner(program); final RelRoot root = tester.convertSqlToRel(sql); final RelNode relInitial = root.rel; @@ -1629,7 +1653,7 @@ public class RelOptRulesTest extends RelOptTestBase { .addRuleInstance(JoinPushTransitivePredicatesRule.INSTANCE) .addRuleCollection(Arrays.asList(extraRules)) .build(); - HepPlanner planner2 = new HepPlanner(program2); + final HepPlanner planner2 = new HepPlanner(program2); planner.registerMetadataProviders(list); planner2.setRoot(relBefore); RelNode relAfter = planner2.findBestExp(); http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/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 45ea111..49ef891 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -174,6 +174,40 @@ LogicalProject(EXPR$0=[1]) ]]> </Resource> </TestCase> + <TestCase name="testEmptyAggregate"> + <Resource name="sql"> + <![CDATA[select sum(empno) from emp where false group by deptno]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(EXPR$0=[$1]) + LogicalAggregate(group=[{0}], EXPR$0=[SUM($1)]) + LogicalValues(tuples=[[]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalValues(tuples=[[]]) +]]> + </Resource> + </TestCase> + <TestCase name="testEmptyAggregateEmptyKey"> + <Resource name="sql"> + <![CDATA[select sum(empno) from emp where false]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalAggregate(group=[{}], EXPR$0=[SUM($0)]) + LogicalValues(tuples=[[]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalAggregate(group=[{}], EXPR$0=[SUM($0)]) + LogicalValues(tuples=[[]]) +]]> + </Resource> + </TestCase> <TestCase name="testPushFilterThroughOuterJoin"> <Resource name="sql"> <![CDATA[select 1 from sales.dept d left outer join sales.emp e on d.deptno = e.deptno where d.name = 'Charlie']]>
