This is an automated email from the ASF dual-hosted git repository. danny0405 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 59c7acd802a885dec1db576974bea6e205d3d955 Author: winifredtamg <[email protected]> AuthorDate: Thu Jun 20 11:39:12 2019 +0800 [CALCITE-3136] Fix the default rule description of ConverterRule (TANG Wen-hui) The ConverterRule's description will decide how it is matched, which should be kept synced with the doc and the current implementation. close apache/calcite#1275 --- .../java/org/apache/calcite/plan/RelOptRule.java | 6 +- .../apache/calcite/rel/convert/ConverterRule.java | 2 +- .../org/apache/calcite/plan/RelOptUtilTest.java | 78 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptRule.java b/core/src/main/java/org/apache/calcite/plan/RelOptRule.java index e93c030..c737359 100644 --- a/core/src/main/java/org/apache/calcite/plan/RelOptRule.java +++ b/core/src/main/java/org/apache/calcite/plan/RelOptRule.java @@ -100,7 +100,7 @@ public abstract class RelOptRule { if (description == null) { description = guessDescription(getClass().getName()); } - if (!description.matches("[A-Za-z][-A-Za-z0-9_.():]*")) { + if (!description.matches("[A-Za-z][-A-Za-z0-9_.(),\\[\\]\\s:]*")) { throw new RuntimeException("Rule description '" + description + "' is not valid"); } @@ -536,8 +536,8 @@ public abstract class RelOptRule { * Returns the description of this rule. * * <p>It must be unique (for rules that are not equal) and must consist of - * only the characters A-Z, a-z, 0-9, '_', '.', '(', ')'. It must start with - * a letter. */ + * only the characters A-Z, a-z, 0-9, '_', '.', '(', ')', '-', ',', '[', ']', ':', ' '. + * It must start with a letter. */ public final String toString() { return description; } diff --git a/core/src/main/java/org/apache/calcite/rel/convert/ConverterRule.java b/core/src/main/java/org/apache/calcite/rel/convert/ConverterRule.java index 84094fc..ba5fe19 100644 --- a/core/src/main/java/org/apache/calcite/rel/convert/ConverterRule.java +++ b/core/src/main/java/org/apache/calcite/rel/convert/ConverterRule.java @@ -78,7 +78,7 @@ public abstract class ConverterRule extends RelOptRule { super(convertOperand(clazz, predicate, in), relBuilderFactory, description == null - ? "ConverterRule<in=" + in + ",out=" + out + ">" + ? "ConverterRule(in:" + in + ",out:" + out + ")" : description); this.inTrait = Objects.requireNonNull(in); this.outTrait = Objects.requireNonNull(out); diff --git a/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java b/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java index a284e10..1323e6c 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java @@ -16,7 +16,15 @@ */ package org.apache.calcite.plan; +import org.apache.calcite.rel.RelCollation; +import org.apache.calcite.rel.RelCollationTraitDef; +import org.apache.calcite.rel.RelCollations; +import org.apache.calcite.rel.RelDistribution; +import org.apache.calcite.rel.RelDistributionTraitDef; +import org.apache.calcite.rel.RelDistributions; +import org.apache.calcite.rel.RelFieldCollation; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.convert.ConverterRule; import org.apache.calcite.rel.core.Join; import org.apache.calcite.rel.core.JoinRelType; import org.apache.calcite.rel.core.Project; @@ -37,6 +45,7 @@ import org.apache.calcite.tools.RelBuilder; import org.apache.calcite.util.TestUtil; import org.apache.calcite.util.Util; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -143,6 +152,75 @@ public class RelOptUtilTest { } } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-3136";>[CALCITE-3136] + * Fix the default rule description of ConverterRule</a>. */ + @Test public void testConvertRuleDefaultRuleDescription() { + RelCollation collation1 = + RelCollations.of(new RelFieldCollation(4, RelFieldCollation.Direction.DESCENDING)); + RelCollation collation2 = + RelCollations.of(new RelFieldCollation(0, RelFieldCollation.Direction.DESCENDING)); + RelDistribution distribution1 = RelDistributions.hash(ImmutableList.of(0, 1)); + RelDistribution distribution2 = RelDistributions.range(ImmutableList.of()); + RelOptRule collationConvertRule = new ConverterRule(RelNode.class, + collation1, + collation2, + null) { + @Override public RelNode convert(RelNode rel) { + return null; + } + }; + RelOptRule distributionConvertRule = new ConverterRule(RelNode.class, + distribution1, + distribution2, + null) { + @Override public RelNode convert(RelNode rel) { + return null; + } + }; + RelOptRule compositeConvertRule = new ConverterRule(RelNode.class, + RelCompositeTrait.of(RelCollationTraitDef.INSTANCE, + ImmutableList.of(collation2, collation1)), + RelCompositeTrait.of(RelCollationTraitDef.INSTANCE, + ImmutableList.of(collation1)), + null) { + @Override public RelNode convert(RelNode rel) { + return null; + } + }; + RelOptRule compositeConvertRule0 = new ConverterRule(RelNode.class, + RelCompositeTrait.of(RelDistributionTraitDef.INSTANCE, + ImmutableList.of(distribution1, distribution2)), + RelCompositeTrait.of(RelDistributionTraitDef.INSTANCE, + ImmutableList.of(distribution1)), + null) { + @Override public RelNode convert(RelNode rel) { + return null; + } + }; + assertEquals("ConverterRule(in:[4 DESC],out:[0 DESC])", collationConvertRule.toString()); + assertEquals("ConverterRule(in:hash[0, 1],out:range)", distributionConvertRule.toString()); + assertEquals("ConverterRule(in:[[0 DESC], [4 DESC]],out:[4 DESC])", + compositeConvertRule.toString()); + assertEquals("ConverterRule(in:[hash[0, 1], range],out:hash[0, 1])", + compositeConvertRule0.toString()); + try { + Util.discard( + new ConverterRule(RelNode.class, + new Convention.Impl("{sourceConvention}", RelNode.class), + new Convention.Impl("<targetConvention>", RelNode.class), + null) { + @Override public RelNode convert(RelNode rel) { + return null; + } }); + fail("expected exception"); + } catch (RuntimeException e) { + assertEquals( + "Rule description 'ConverterRule(in:{sourceConvention},out:<targetConvention>)' is not valid", + e.getMessage()); + } + } + /** * Test {@link RelOptUtil#splitJoinCondition(RelNode, RelNode, RexNode, List, List, List)} * where the join condition contains just one which is a EQUAL operator.
