This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 93c305031fb6c152c118390647f2b9b0979109c2 Author: devozerov <[email protected]> AuthorDate: Fri Feb 26 14:38:30 2021 +0300 [CALCITE-4515] Do not generate the new join tree from commute/associate rules if there are "always TRUE" conditions (Vladimir Ozerov) --- .../calcite/rel/rules/JoinAssociateRule.java | 44 +++++-- .../apache/calcite/rel/rules/JoinCommuteRule.java | 19 ++- .../org/apache/calcite/test/RelOptRulesTest.java | 138 +++++++++++++++++++++ .../org/apache/calcite/test/RelOptRulesTest.xml | 67 +++++++++- 4 files changed, 253 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java b/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java index a4eb516..d65dfa3 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/JoinAssociateRule.java @@ -19,7 +19,6 @@ package org.apache.calcite.rel.rules; import org.apache.calcite.plan.RelOptCluster; import org.apache.calcite.plan.RelOptRuleCall; import org.apache.calcite.plan.RelRule; -import org.apache.calcite.plan.volcano.RelSubset; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.core.Join; import org.apache.calcite.rel.core.JoinRelType; @@ -28,6 +27,7 @@ import org.apache.calcite.rex.RexNode; import org.apache.calcite.rex.RexPermuteInputsShuttle; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.tools.RelBuilderFactory; +import org.apache.calcite.util.ImmutableBeans; import org.apache.calcite.util.ImmutableBitSet; import org.apache.calcite.util.mapping.Mappings; @@ -68,7 +68,7 @@ public class JoinAssociateRule final Join bottomJoin = call.rel(1); final RelNode relA = bottomJoin.getLeft(); final RelNode relB = bottomJoin.getRight(); - final RelSubset relC = call.rel(2); + final RelNode relC = call.rel(2); final RelOptCluster cluster = topJoin.getCluster(); final RexBuilder rexBuilder = cluster.getRexBuilder(); @@ -120,6 +120,11 @@ public class JoinAssociateRule JoinPushThroughJoinRule.split(bottomJoin.getCondition(), aBitSet, top, bottom); + final boolean allowAlwaysTrueCondition = config.isAllowAlwaysTrueCondition(); + if (!allowAlwaysTrueCondition && (top.isEmpty() || bottom.isEmpty())) { + return; + } + // Mapping for moving conditions from topJoin or bottomJoin to // newBottomJoin. // target: | B | C | @@ -132,16 +137,23 @@ public class JoinAssociateRule final List<RexNode> newBottomList = new RexPermuteInputsShuttle(bottomMapping, relB, relC) .visitList(bottom); - RexNode newBottomCondition = - RexUtil.composeConjunction(rexBuilder, newBottomList); - final Join newBottomJoin = - bottomJoin.copy(bottomJoin.getTraitSet(), newBottomCondition, relB, - relC, JoinRelType.INNER, false); + RexNode newBottomCondition = RexUtil.composeConjunction(rexBuilder, newBottomList); + if (!allowAlwaysTrueCondition && newBottomCondition.isAlwaysTrue()) { + return; + } // Condition for newTopJoin consists of pieces from bottomJoin and topJoin. // Field ordinals do not need to be changed. RexNode newTopCondition = RexUtil.composeConjunction(rexBuilder, top); + if (!allowAlwaysTrueCondition && newTopCondition.isAlwaysTrue()) { + return; + } + + final Join newBottomJoin = + bottomJoin.copy(bottomJoin.getTraitSet(), newBottomCondition, relB, + relC, JoinRelType.INNER, false); + @SuppressWarnings("SuspiciousNameCombination") final Join newTopJoin = topJoin.copy(topJoin.getTraitSet(), newTopCondition, relA, @@ -153,19 +165,29 @@ public class JoinAssociateRule /** Rule configuration. */ public interface Config extends RelRule.Config { Config DEFAULT = EMPTY.as(Config.class) - .withOperandFor(Join.class, RelSubset.class); + .withOperandFor(Join.class); @Override default JoinAssociateRule toRule() { return new JoinAssociateRule(this); } + /** + * Whether to emit the new join tree if the new top or bottom join has a condition which + * is always {@code TRUE}. + */ + @ImmutableBeans.Property + @ImmutableBeans.BooleanDefault(true) + boolean isAllowAlwaysTrueCondition(); + + /** Sets {@link #isAllowAlwaysTrueCondition()}. */ + Config withAllowAlwaysTrueCondition(boolean allowAlwaysTrueCondition); + /** Defines an operand tree for the given classes. */ - default Config withOperandFor(Class<? extends Join> joinClass, - Class<? extends RelSubset> relSubsetClass) { + default Config withOperandFor(Class<? extends Join> joinClass) { return withOperandSupplier(b0 -> b0.operand(joinClass).inputs( b1 -> b1.operand(joinClass).anyInputs(), - b2 -> b2.operand(relSubsetClass).anyInputs())) + b2 -> b2.operand(RelNode.class).anyInputs())) .as(Config.class); } } diff --git a/core/src/main/java/org/apache/calcite/rel/rules/JoinCommuteRule.java b/core/src/main/java/org/apache/calcite/rel/rules/JoinCommuteRule.java index ee4752e..c38be27 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/JoinCommuteRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/JoinCommuteRule.java @@ -139,7 +139,13 @@ public class JoinCommuteRule @Override public boolean matches(RelOptRuleCall call) { Join join = call.rel(0); // SEMI and ANTI join cannot be swapped. - return join.getJoinType().projectsRight(); + if (!join.getJoinType().projectsRight()) { + return false; + } + + // Suppress join with "true" condition (that is, cartesian joins). + return config.isAllowAlwaysTrueCondition() + || !join.getCondition().isAlwaysTrue(); } @Override public void onMatch(final RelOptRuleCall call) { @@ -241,12 +247,21 @@ public class JoinCommuteRule .as(Config.class); } - /** Whether to swap outer joins. */ + /** Whether to swap outer joins; default false. */ @ImmutableBeans.Property @ImmutableBeans.BooleanDefault(false) boolean isSwapOuter(); /** Sets {@link #isSwapOuter()}. */ Config withSwapOuter(boolean swapOuter); + + /** Whether to emit the new join tree if the join condition is {@code TRUE} + * (that is, cartesian joins); default true. */ + @ImmutableBeans.Property + @ImmutableBeans.BooleanDefault(true) + boolean isAllowAlwaysTrueCondition(); + + /** Sets {@link #isAllowAlwaysTrueCondition()}. */ + Config withAllowAlwaysTrueCondition(boolean allowAlwaysTrueCondition); } } 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 449914b..6ef31a5 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -71,6 +71,8 @@ import org.apache.calcite.rel.rules.DateRangeRules; import org.apache.calcite.rel.rules.FilterJoinRule; import org.apache.calcite.rel.rules.FilterMultiJoinMergeRule; import org.apache.calcite.rel.rules.FilterProjectTransposeRule; +import org.apache.calcite.rel.rules.JoinAssociateRule; +import org.apache.calcite.rel.rules.JoinCommuteRule; import org.apache.calcite.rel.rules.MultiJoin; import org.apache.calcite.rel.rules.ProjectCorrelateTransposeRule; import org.apache.calcite.rel.rules.ProjectFilterTransposeRule; @@ -6981,4 +6983,140 @@ class RelOptRulesTest extends RelOptTestBase { .withRule(CoreRules.FILTER_REDUCE_EXPRESSIONS) .checkUnchanged(); } + + @Test void testJoinCommuteRuleWithAlwaysTrueConditionAllowed() { + checkJoinCommuteRuleWithAlwaysTrueConditionDisallowed(true); + } + + @Test void testJoinCommuteRuleWithAlwaysTrueConditionDisallowed() { + checkJoinCommuteRuleWithAlwaysTrueConditionDisallowed(false); + } + + private void checkJoinCommuteRuleWithAlwaysTrueConditionDisallowed(boolean allowAlwaysTrue) { + final RelBuilder relBuilder = RelBuilder.create(RelBuilderTest.config().build()); + + RelNode left = relBuilder.scan("EMP").build(); + RelNode right = relBuilder.scan("DEPT").build(); + + RelNode relNode = relBuilder.push(left) + .push(right) + .join(JoinRelType.INNER, + relBuilder.literal(true)) + .build(); + + JoinCommuteRule.Config ruleConfig = JoinCommuteRule.Config.DEFAULT; + if (!allowAlwaysTrue) { + ruleConfig = ruleConfig.withAllowAlwaysTrueCondition(false); + } + + HepProgram program = new HepProgramBuilder() + .addMatchLimit(1) + .addRuleInstance(ruleConfig.toRule()) + .build(); + + HepPlanner hepPlanner = new HepPlanner(program); + hepPlanner.setRoot(relNode); + RelNode output = hepPlanner.findBestExp(); + + final String planAfter = NL + RelOptUtil.toString(output); + final DiffRepository diffRepos = getDiffRepos(); + diffRepos.assertEquals("planAfter", "${planAfter}", planAfter); + SqlToRelTestBase.assertValid(output); + } + + @Test void testJoinAssociateRuleWithBottomAlwaysTrueConditionAllowed() { + checkJoinAssociateRuleWithBottomAlwaysTrueCondition(true); + } + + @Test void testJoinAssociateRuleWithBottomAlwaysTrueConditionDisallowed() { + checkJoinAssociateRuleWithBottomAlwaysTrueCondition(false); + } + + private void checkJoinAssociateRuleWithBottomAlwaysTrueCondition(boolean allowAlwaysTrue) { + final RelBuilder relBuilder = RelBuilder.create(RelBuilderTest.config().build()); + + RelNode bottomLeft = relBuilder.scan("EMP").build(); + RelNode bottomRight = relBuilder.scan("DEPT").build(); + RelNode top = relBuilder.scan("BONUS").build(); + + RelNode relNode = relBuilder.push(bottomLeft) + .push(bottomRight) + .join(JoinRelType.INNER, + relBuilder.call(SqlStdOperatorTable.EQUALS, + relBuilder.field(2, 0, "DEPTNO"), + relBuilder.field(2, 1, "DEPTNO"))) + .push(top) + .join(JoinRelType.INNER, + relBuilder.call(SqlStdOperatorTable.EQUALS, + relBuilder.field(2, 0, "JOB"), + relBuilder.field(2, 1, "JOB"))) + .build(); + + JoinAssociateRule.Config ruleConfig = JoinAssociateRule.Config.DEFAULT; + if (!allowAlwaysTrue) { + ruleConfig = ruleConfig.withAllowAlwaysTrueCondition(false); + } + + HepProgram program = new HepProgramBuilder() + .addMatchLimit(1) + .addMatchOrder(HepMatchOrder.TOP_DOWN) + .addRuleInstance(ruleConfig.toRule()) + .build(); + + HepPlanner hepPlanner = new HepPlanner(program); + hepPlanner.setRoot(relNode); + RelNode output = hepPlanner.findBestExp(); + + final String planAfter = NL + RelOptUtil.toString(output); + final DiffRepository diffRepos = getDiffRepos(); + diffRepos.assertEquals("planAfter", "${planAfter}", planAfter); + SqlToRelTestBase.assertValid(output); + } + + @Test void testJoinAssociateRuleWithTopAlwaysTrueConditionAllowed() { + checkJoinAssociateRuleWithTopAlwaysTrueCondition(true); + } + + @Test void testJoinAssociateRuleWithTopAlwaysTrueConditionDisallowed() { + checkJoinAssociateRuleWithTopAlwaysTrueCondition(false); + } + + private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlwaysTrue) { + final RelBuilder relBuilder = RelBuilder.create(RelBuilderTest.config().build()); + + RelNode bottomLeft = relBuilder.scan("EMP").build(); + RelNode bottomRight = relBuilder.scan("BONUS").build(); + RelNode top = relBuilder.scan("DEPT").build(); + + RelNode relNode = relBuilder.push(bottomLeft) + .push(bottomRight) + .join(JoinRelType.INNER, + relBuilder.literal(true)) + .push(top) + .join(JoinRelType.INNER, + relBuilder.call(SqlStdOperatorTable.EQUALS, + relBuilder.field(2, 0, "DEPTNO"), + relBuilder.field(2, 1, "DEPTNO"))) + .build(); + + JoinAssociateRule.Config ruleConfig = JoinAssociateRule.Config.DEFAULT; + if (!allowAlwaysTrue) { + ruleConfig = ruleConfig.withAllowAlwaysTrueCondition(false); + } + + HepProgram program = new HepProgramBuilder() + .addMatchLimit(1) + .addMatchOrder(HepMatchOrder.TOP_DOWN) + .addRuleInstance(ruleConfig.toRule()) + .build(); + + HepPlanner hepPlanner = new HepPlanner(program); + hepPlanner.setRoot(relNode); + RelNode output = hepPlanner.findBestExp(); + + final String planAfter = NL + RelOptUtil.toString(output); + final DiffRepository diffRepos = getDiffRepos(); + diffRepos.assertEquals("planAfter", "${planAfter}", planAfter); + SqlToRelTestBase.assertValid(output); + } } 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 8346c92..3dc6ad2 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -3718,6 +3718,69 @@ LogicalIntersect(all=[true]) ]]> </Resource> </TestCase> + <TestCase name="testJoinAssociateRuleWithBottomAlwaysTrueConditionAllowed"> + <Resource name="planAfter"> + <![CDATA[ +LogicalJoin(condition=[AND(=($2, $12), =($7, $8))], joinType=[inner]) + LogicalTableScan(table=[[scott, EMP]]) + LogicalJoin(condition=[true], joinType=[inner]) + LogicalTableScan(table=[[scott, DEPT]]) + LogicalTableScan(table=[[scott, BONUS]]) +]]> + </Resource> + </TestCase> + <TestCase name="testJoinAssociateRuleWithBottomAlwaysTrueConditionDisallowed"> + <Resource name="planAfter"> + <![CDATA[ +LogicalJoin(condition=[=($2, $12)], joinType=[inner]) + LogicalJoin(condition=[=($7, $8)], joinType=[inner]) + LogicalTableScan(table=[[scott, EMP]]) + LogicalTableScan(table=[[scott, DEPT]]) + LogicalTableScan(table=[[scott, BONUS]]) +]]> + </Resource> + </TestCase> + <TestCase name="testJoinAssociateRuleWithTopAlwaysTrueConditionAllowed"> + <Resource name="planAfter"> + <![CDATA[ +LogicalJoin(condition=[=($7, $12)], joinType=[inner]) + LogicalTableScan(table=[[scott, EMP]]) + LogicalJoin(condition=[true], joinType=[inner]) + LogicalTableScan(table=[[scott, BONUS]]) + LogicalTableScan(table=[[scott, DEPT]]) +]]> + </Resource> + </TestCase> + <TestCase name="testJoinAssociateRuleWithTopAlwaysTrueConditionDisallowed"> + <Resource name="planAfter"> + <![CDATA[ +LogicalJoin(condition=[=($7, $12)], joinType=[inner]) + LogicalJoin(condition=[true], joinType=[inner]) + LogicalTableScan(table=[[scott, EMP]]) + LogicalTableScan(table=[[scott, BONUS]]) + LogicalTableScan(table=[[scott, DEPT]]) +]]> + </Resource> + </TestCase> + <TestCase name="testJoinCommuteRuleWithAlwaysTrueConditionAllowed"> + <Resource name="planAfter"> + <![CDATA[ +LogicalProject(EMPNO=[$3], ENAME=[$4], JOB=[$5], MGR=[$6], HIREDATE=[$7], SAL=[$8], COMM=[$9], DEPTNO=[$10], DEPTNO0=[$0], DNAME=[$1], LOC=[$2]) + LogicalJoin(condition=[true], joinType=[inner]) + LogicalTableScan(table=[[scott, DEPT]]) + LogicalTableScan(table=[[scott, EMP]]) +]]> + </Resource> + </TestCase> + <TestCase name="testJoinCommuteRuleWithAlwaysTrueConditionDisallowed"> + <Resource name="planAfter"> + <![CDATA[ +LogicalJoin(condition=[true], joinType=[inner]) + LogicalTableScan(table=[[scott, EMP]]) + LogicalTableScan(table=[[scott, DEPT]]) +]]> + </Resource> + </TestCase> <TestCase name="testJoinProjectTransposeWindow"> <Resource name="sql"> <![CDATA[select * @@ -8227,7 +8290,7 @@ LogicalAggregate(group=[{}], EXPR$0=[SUM($0)]) </TestCase> <TestCase name="testPushAggregateThroughJoin7"> <Resource name="sql"> - <![CDATA[select any_value(B.sal) + <![CDATA[select any_value(distinct B.sal) from sales.emp as A join (select distinct sal from sales.emp) as B on A.sal=B.sal @@ -8255,7 +8318,7 @@ LogicalAggregate(group=[{}], EXPR$0=[ANY_VALUE($1)]) </TestCase> <TestCase name="testPushAggregateThroughJoin8"> <Resource name="sql"> - <