[CALCITE-2108] AggregateJoinTransposeRule incorrectly splits a SUM0 call when Aggregate has no group keys (jingzhang)
Close apache/calcite#591 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/cb8376b1 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/cb8376b1 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/cb8376b1 Branch: refs/heads/master Commit: cb8376b13ad50003134e398a87161bec68908606 Parents: a609b47 Author: beyond1920 <[email protected]> Authored: Fri Dec 22 17:56:44 2017 +0800 Committer: Julian Hyde <[email protected]> Committed: Sun Dec 24 21:09:41 2017 -0800 ---------------------------------------------------------------------- .../calcite/sql/SqlSplittableAggFunction.java | 28 ++++++++++++++--- .../sql/fun/SqlSumEmptyIsZeroAggFunction.java | 2 +- .../apache/calcite/test/RelOptRulesTest.java | 21 +++++++++++++ .../org/apache/calcite/test/RelOptRulesTest.xml | 32 ++++++++++++++++++++ 4 files changed, 78 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/cb8376b1/core/src/main/java/org/apache/calcite/sql/SqlSplittableAggFunction.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/SqlSplittableAggFunction.java b/core/src/main/java/org/apache/calcite/sql/SqlSplittableAggFunction.java index 6e8e4fe..83dad3a 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlSplittableAggFunction.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlSplittableAggFunction.java @@ -212,9 +212,8 @@ public interface SqlSplittableAggFunction { } } - /** Splitting strategy for {@code SUM}. */ - class SumSplitter implements SqlSplittableAggFunction { - public static final SumSplitter INSTANCE = new SumSplitter(); + /** Common splitting strategy for {@code SUM} and {@code SUM0} functions. */ + abstract class AbstractSumSplitter implements SqlSplittableAggFunction { public RexNode singleton(RexBuilder rexBuilder, RelDataType inputRowType, AggregateCall aggregateCall) { @@ -260,10 +259,31 @@ public interface SqlSplittableAggFunction { throw new AssertionError("unexpected count " + merges); } int ordinal = extra.register(node); - return AggregateCall.create(SqlStdOperatorTable.SUM, false, false, + return AggregateCall.create(getMergeAggFunctionOfTopSplit(), false, false, ImmutableList.of(ordinal), -1, aggregateCall.type, aggregateCall.name); } + + protected abstract SqlAggFunction getMergeAggFunctionOfTopSplit(); + + } + + /** Splitting strategy for {@code SUM} function. */ + class SumSplitter extends AbstractSumSplitter { + public static final SumSplitter INSTANCE = new SumSplitter(); + + @Override public SqlAggFunction getMergeAggFunctionOfTopSplit() { + return SqlStdOperatorTable.SUM; + } + } + + /** Splitting strategy for {@code SUM0} function. */ + class Sum0Splitter extends AbstractSumSplitter { + public static final Sum0Splitter INSTANCE = new Sum0Splitter(); + + @Override public SqlAggFunction getMergeAggFunctionOfTopSplit() { + return SqlStdOperatorTable.SUM0; + } } } http://git-wip-us.apache.org/repos/asf/calcite/blob/cb8376b1/core/src/main/java/org/apache/calcite/sql/fun/SqlSumEmptyIsZeroAggFunction.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlSumEmptyIsZeroAggFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlSumEmptyIsZeroAggFunction.java index daa66e2..1fd9bf1 100644 --- a/core/src/main/java/org/apache/calcite/sql/fun/SqlSumEmptyIsZeroAggFunction.java +++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlSumEmptyIsZeroAggFunction.java @@ -68,7 +68,7 @@ public class SqlSumEmptyIsZeroAggFunction extends SqlAggFunction { @Override public <T> T unwrap(Class<T> clazz) { if (clazz == SqlSplittableAggFunction.class) { - return clazz.cast(SqlSplittableAggFunction.SumSplitter.INSTANCE); + return clazz.cast(SqlSplittableAggFunction.Sum0Splitter.INSTANCE); } return super.unwrap(clazz); } http://git-wip-us.apache.org/repos/asf/calcite/blob/cb8376b1/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 9ec3918..321d9fc 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -2914,6 +2914,27 @@ public class RelOptRulesTest extends RelOptTestBase { checkPlanning(tester, preProgram, new HepPlanner(program), sql); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-2108";>[CALCITE-2108] + * AggregateJoinTransposeRule incorrectly splits a SUM0 call when Aggregate + * has no group keys</a>. + * + * <p>Similar to {@link #testPushAggregateSumThroughJoin()}, + * but also uses {@link AggregateReduceFunctionsRule}. */ + @Test public void testPushAggregateSumThroughJoinAfterAggregateReduce() { + final HepProgram preProgram = new HepProgramBuilder() + .addRuleInstance(AggregateProjectMergeRule.INSTANCE) + .build(); + final HepProgram program = new HepProgramBuilder() + .addRuleInstance(AggregateReduceFunctionsRule.INSTANCE) + .addRuleInstance(AggregateJoinTransposeRule.EXTENDED) + .build(); + final String sql = "select sum(sal)\n" + + "from (select * from sales.emp where empno = 10) as e\n" + + "join sales.dept as d on e.job = d.name"; + checkPlanning(tester, preProgram, new HepPlanner(program), sql); + } + /** Push a variety of aggregate functions. */ @Test public void testPushAggregateFunctionsThroughJoin() { final HepProgram preProgram = new HepProgramBuilder() http://git-wip-us.apache.org/repos/asf/calcite/blob/cb8376b1/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 fa4a2a5..46518f9 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -5754,6 +5754,38 @@ LogicalAggregate(group=[{}], EXPR$0=[SUM($4)]) ]]> </Resource> </TestCase> + <TestCase name="testPushAggregateSumThroughJoinAfterAggregateReduce"> + <Resource name="sql"> + <![CDATA[select e.job,sum(sal) +from (select * from sales.emp where empno = 10) as e +join sales.dept as d on e.job = d.name +group by e.job,d.name]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalAggregate(group=[{}], EXPR$0=[SUM($5)]) + LogicalJoin(condition=[=($2, $10)], joinType=[inner]) + LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8]) + LogicalFilter(condition=[=($0, 10)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(EXPR$0=[CASE(=($1, 0), null, $0)]) + LogicalAggregate(group=[{}], EXPR$0=[$SUM0($5)], agg#1=[$SUM0($6)]) + LogicalProject(JOB=[$0], EXPR$0=[$1], $f2=[$2], NAME=[$3], $f1=[$4], $f5=[CAST(*($1, $4)):INTEGER NOT NULL], $f6=[*($2, $4)]) + LogicalJoin(condition=[=($0, $3)], joinType=[inner]) + LogicalAggregate(group=[{2}], EXPR$0=[$SUM0($5)], agg#1=[COUNT()]) + LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8]) + LogicalFilter(condition=[=($0, 10)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalAggregate(group=[{1}], agg#0=[COUNT()]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + </TestCase> <TestCase name="testReduceConstantsIsNotNull"> <Resource name="sql"> <