You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by da...@apache.org on 2019/07/22 02:35:30 UTC
[calcite] 01/01: [CALCITE-3136] Fix the default rule description of
ConverterRule (TANG Wen-hui)
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 <we...@yeah.net>
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.