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:29 UTC

[calcite] branch master updated (477937b -> 59c7acd)

This is an automated email from the ASF dual-hosted git repository.

danny0405 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git.


    omit 477937b  [CALCITE-3136]Fix the default rule description of ConverterRule (TANG Wen-hui)
     new 59c7acd  [CALCITE-3136] Fix the default rule description of ConverterRule (TANG Wen-hui)

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (477937b)
            \
             N -- N -- N   refs/heads/master (59c7acd)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:


[calcite] 01/01: [CALCITE-3136] Fix the default rule description of ConverterRule (TANG Wen-hui)

Posted by da...@apache.org.
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.