mihaibudiu commented on code in PR #5029: URL: https://github.com/apache/calcite/pull/5029#discussion_r3445024770
########## core/src/main/java/org/apache/calcite/rel/rules/JoinAggregateTransposeRule.java: ########## @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.rel.rules; + +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelRule; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Aggregate; +import org.apache.calcite.rel.core.Join; +import org.apache.calcite.rel.core.JoinInfo; +import org.apache.calcite.rel.core.JoinRelType; +import org.apache.calcite.rel.metadata.RelMetadataQuery; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexUtil; +import org.apache.calcite.tools.RelBuilder; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.calcite.util.mapping.Mappings; + +import org.immutables.value.Value; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.apache.calcite.rel.rules.AggregateJoinTransposeRule.isAggregateSupported; + +/** + * Planner rule that pulls an + * {@link org.apache.calcite.rel.core.Aggregate} + * from below a {@link org.apache.calcite.rel.core.Join} to above it. + * + * <p>This rule implements the group-by pull up transformation and is described + * in the following papers: + * + * <ul> + * <li>Weipeng P. Yan, and Per-Ake Larson. "Interchanging the order of grouping and join". Technical + * Report CS 95-09, Dept. of Computer Science, University of Waterloo, Canada, 1995.</li> + * <li>Weipeng P. Yan, and Per-Ake Larson. "Eager Aggregation and Lazy Aggregation." Proceedings + * of the 21th International Conference on Very Large Data Bases. 1995.</li> + * </ul> Review Comment: A short summary of what the rule does will help readers who are too lazy to find the papers. ########## core/src/test/resources/org/apache/calcite/test/JoinAggregateTransposeRuleTest.xml: ########## @@ -0,0 +1,164 @@ +<?xml version="1.0" ?> +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one or more + ~ contributor license agreements. See the NOTICE file distributed with + ~ this work for additional information regarding copyright ownership. + ~ The ASF licenses this file to you under the Apache License, Version 2.0 + ~ (the "License"); you may not use this file except in compliance with + ~ the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> +<Root> + <TestCase name="testNoPullGroupByAboveNonEquiJoin"> + <Resource name="sql"> + <![CDATA[select g.deptno +from (select deptno from sales.emp group by deptno) g +join sales.dept d on g.deptno > d.deptno]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(DEPTNO=[$0]) + LogicalJoin(condition=[>($0, $1)], joinType=[inner]) + LogicalAggregate(group=[{7}]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + </TestCase> + <TestCase name="testNoPullGroupByWhenRightJoinKeysNotUnique"> + <Resource name="sql"> + <![CDATA[select g.job, g.cnt +from (select job, count(*) as cnt + from sales.emp group by job) g +join sales.emp e on g.job = e.job]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(JOB=[$0], CNT=[$1]) + LogicalJoin(condition=[=($0, $4)], joinType=[inner]) + LogicalAggregate(group=[{2}], CNT=[COUNT()]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> + <TestCase name="testPullGroupByFromLeftWithFilterOnRight"> + <Resource name="sql"> + <![CDATA[select g.deptno, g.total_sal, d.name +from (select deptno, sum(sal) as total_sal + from sales.emp group by deptno) g +join sales.dept d on g.deptno = d.deptno +where d.name = 'Engineering']]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$1], NAME=[$3]) + LogicalJoin(condition=[=($0, $2)], joinType=[inner]) + LogicalAggregate(group=[{7}], TOTAL_SAL=[SUM($5)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalFilter(condition=[=(CAST($1):VARCHAR(11) NOT NULL, 'Engineering')]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$1], NAME=[$3]) + LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$3], DEPTNO0=[$1], NAME=[$2]) + LogicalAggregate(group=[{7, 9, 10}], TOTAL_SAL=[SUM($5)]) + LogicalJoin(condition=[=($7, $9)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalFilter(condition=[=(CAST($1):VARCHAR(11) NOT NULL, 'Engineering')]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + </TestCase> + <TestCase name="testPullGroupByFromLeftWithSimpleAggregation"> + <Resource name="sql"> + <![CDATA[select g.deptno, g.total_sal, d.name +from (select deptno, sum(sal) as total_sal + from sales.emp group by deptno) g +join sales.dept d on g.deptno = d.deptno]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$1], NAME=[$3]) + LogicalJoin(condition=[=($0, $2)], joinType=[inner]) + LogicalAggregate(group=[{7}], TOTAL_SAL=[SUM($5)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$1], NAME=[$3]) + LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$3], DEPTNO0=[$1], NAME=[$2]) + LogicalAggregate(group=[{7, 9, 10}], TOTAL_SAL=[SUM($5)]) + LogicalJoin(condition=[=($7, $9)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + </TestCase> + <TestCase name="testPullGroupByFromRightWithSimpleAggregation"> + <Resource name="sql"> + <![CDATA[select g.deptno, g.total_sal, d.name +from sales.dept d +join (select deptno, sum(sal) as total_sal + from sales.emp group by deptno) g + on g.deptno = d.deptno]]> + </Resource> + <Resource name="planBefore"> + <![CDATA[ +LogicalProject(DEPTNO=[$2], TOTAL_SAL=[$3], NAME=[$1]) + LogicalJoin(condition=[=($2, $0)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalAggregate(group=[{0}], TOTAL_SAL=[SUM($1)]) + LogicalProject(DEPTNO=[$7], SAL=[$5]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(DEPTNO=[$2], TOTAL_SAL=[$3], NAME=[$1]) + LogicalProject(DEPTNO=[$2], NAME=[$3], DEPTNO0=[$0], TOTAL_SAL=[$1]) + LogicalProject(DEPTNO=[$0], TOTAL_SAL=[$3], DEPTNO0=[$1], NAME=[$2]) + LogicalAggregate(group=[{7, 9, 10}], TOTAL_SAL=[SUM($5)]) Review Comment: Some quidem tests would increase our confidence. Moreover, I don't see any test with 2 aggregates, the rule seems to allow it ########## core/src/main/java/org/apache/calcite/rel/rules/JoinAggregateTransposeRule.java: ########## @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.rel.rules; + +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelRule; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Aggregate; +import org.apache.calcite.rel.core.Join; +import org.apache.calcite.rel.core.JoinInfo; +import org.apache.calcite.rel.core.JoinRelType; +import org.apache.calcite.rel.metadata.RelMetadataQuery; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexUtil; +import org.apache.calcite.tools.RelBuilder; +import org.apache.calcite.util.ImmutableBitSet; +import org.apache.calcite.util.mapping.Mappings; + +import org.immutables.value.Value; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.apache.calcite.rel.rules.AggregateJoinTransposeRule.isAggregateSupported; + +/** + * Planner rule that pulls an + * {@link org.apache.calcite.rel.core.Aggregate} + * from below a {@link org.apache.calcite.rel.core.Join} to above it. + * + * <p>This rule implements the group-by pull up transformation and is described + * in the following papers: + * + * <ul> + * <li>Weipeng P. Yan, and Per-Ake Larson. "Interchanging the order of grouping and join". Technical + * Report CS 95-09, Dept. of Computer Science, University of Waterloo, Canada, 1995.</li> + * <li>Weipeng P. Yan, and Per-Ake Larson. "Eager Aggregation and Lazy Aggregation." Proceedings + * of the 21th International Conference on Very Large Data Bases. 1995.</li> + * </ul> + * + * @see CoreRules#JOIN_AGGREGATE_TRANSPOSE + */ [email protected] +public class JoinAggregateTransposeRule + extends RelRule<JoinAggregateTransposeRule.Config> + implements TransformationRule { + + protected JoinAggregateTransposeRule(Config config) { + super(config); + } + + @Override public final boolean matches(RelOptRuleCall call) { + final Join join = call.rel(0); + final Aggregate left = call.rel(1); + final JoinInfo info = join.analyzeCondition(); + + // Only handle INNER equijoins with simple aggregates for now. + // Join keys on the agg side must reference only group-by columns + // (ensures row elimination removes whole groups, not partial) + ImmutableBitSet groupOutput = ImmutableBitSet.range(left.getGroupCount()); + return join.getJoinType() == JoinRelType.INNER + && info.isEqui() + // We could potentially relax the check for the supported functions + // in this rule. I opted to keep things more constrained for now + // in case we decide to extend this rule for lazy aggregation. + && isAggregateSupported(left, true) + && groupOutput.contains(info.leftSet()); + } + + @Override public void onMatch(RelOptRuleCall call) { + final Join join = call.rel(0); + final Aggregate left = call.rel(1); + final RelNode aggInput = left.getInput(); + final RelNode right = join.getRight(); + + final JoinInfo joinInfo = join.analyzeCondition(); + // The right side must be unique on its join keys (no row duplication) Review Comment: I imagine this must be symmetric for inner joins, but perhaps you rely on some join commutativity rule to find the symmetric optimization? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
