You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2016/04/22 10:07:34 UTC
[05/13] calcite git commit: [CALCITE-1166] Disallow sub-classes of
RelOptRuleOperand
[CALCITE-1166] Disallow sub-classes of RelOptRuleOperand
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/d81133fd
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/d81133fd
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/d81133fd
Branch: refs/heads/master
Commit: d81133fd0e8aee9dcd80eb62046550f65464b377
Parents: e91c8dc
Author: Julian Hyde <jh...@apache.org>
Authored: Wed Mar 23 16:13:26 2016 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Tue Apr 19 11:11:37 2016 -0700
----------------------------------------------------------------------
.../org/apache/calcite/plan/RelOptRule.java | 58 ++++++++++++++++----
.../apache/calcite/plan/RelOptRuleOperand.java | 37 ++++++++++---
.../calcite/plan/RelOptRuleOperandChildren.java | 6 +-
.../calcite/rel/convert/ConverterRule.java | 25 +--------
4 files changed, 81 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/d81133fd/core/src/main/java/org/apache/calcite/plan/RelOptRule.java
----------------------------------------------------------------------
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 2d5c5d6..4c3765d 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptRule.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptRule.java
@@ -17,6 +17,8 @@
package org.apache.calcite.plan;
import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.convert.Converter;
+import org.apache.calcite.rel.convert.ConverterRule;
import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.tools.RelBuilderFactory;
@@ -125,7 +127,7 @@ public abstract class RelOptRule {
Class<R> clazz,
RelOptRuleOperandChildren operandList) {
return new RelOptRuleOperand(clazz, null, Predicates.<R>alwaysTrue(),
- operandList);
+ operandList.policy, operandList.operands);
}
/**
@@ -144,7 +146,7 @@ public abstract class RelOptRule {
RelTrait trait,
RelOptRuleOperandChildren operandList) {
return new RelOptRuleOperand(clazz, trait, Predicates.<R>alwaysTrue(),
- operandList);
+ operandList.policy, operandList.operands);
}
/**
@@ -164,7 +166,8 @@ public abstract class RelOptRule {
RelTrait trait,
Predicate<? super R> predicate,
RelOptRuleOperandChildren operandList) {
- return new RelOptRuleOperand(clazz, trait, predicate, operandList);
+ return new RelOptRuleOperand(clazz, trait, predicate, operandList.policy,
+ operandList.operands);
}
/**
@@ -185,7 +188,7 @@ public abstract class RelOptRule {
Predicate<? super R> predicate,
RelOptRuleOperand first,
RelOptRuleOperand... rest) {
- return new RelOptRuleOperand(clazz, trait, predicate, some(first, rest));
+ return operand(clazz, trait, predicate, some(first, rest));
}
/**
@@ -214,6 +217,18 @@ public abstract class RelOptRule {
return operand(clazz, some(first, rest));
}
+ /**
+ * Creates an operand for a converter rule.
+ *
+ * @param clazz Class of relational expression to match (must not be null)
+ * @param trait Trait to match, or null to match any trait
+ * @param predicate Predicate to apply to relational expression
+ */
+ protected static <R extends RelNode> ConverterRelOptRuleOperand
+ convertOperand(Class<R> clazz, Predicate<? super R> predicate,
+ RelTrait trait) {
+ return new ConverterRelOptRuleOperand(clazz, trait, predicate);
+ }
//~ Methods for creating lists of child operands ---------------------------
@@ -229,10 +244,8 @@ public abstract class RelOptRule {
public static RelOptRuleOperandChildren some(
RelOptRuleOperand first,
RelOptRuleOperand... rest) {
- return new RelOptRuleOperandChildren(
- RelOptRuleOperandChildPolicy.SOME,
- ImmutableList.<RelOptRuleOperand>builder().add(first)
- .add(rest).build());
+ return new RelOptRuleOperandChildren(RelOptRuleOperandChildPolicy.SOME,
+ Lists.asList(first, rest));
}
@@ -267,8 +280,7 @@ public abstract class RelOptRule {
RelOptRuleOperand... rest) {
return new RelOptRuleOperandChildren(
RelOptRuleOperandChildPolicy.UNORDERED,
- ImmutableList.<RelOptRuleOperand>builder().add(first)
- .add(rest).build());
+ Lists.asList(first, rest));
}
/**
@@ -557,8 +569,7 @@ public abstract class RelOptRule {
* @param trait Trait to add to each relational expression
* @return List of converted relational expressions, never null
*/
- public static List<RelNode> convertList(
- List<RelNode> rels,
+ protected static List<RelNode> convertList(List<RelNode> rels,
final RelTrait trait) {
return Lists.transform(rels,
new Function<RelNode, RelNode>() {
@@ -601,6 +612,29 @@ public abstract class RelOptRule {
return description;
}
+ /**
+ * Operand to an instance of the converter rule.
+ */
+ private static class ConverterRelOptRuleOperand extends RelOptRuleOperand {
+ <R extends RelNode> ConverterRelOptRuleOperand(Class<R> clazz, RelTrait in,
+ Predicate<? super R> predicate) {
+ super(clazz, in, predicate, RelOptRuleOperandChildPolicy.ANY,
+ ImmutableList.<RelOptRuleOperand>of());
+ }
+
+ public boolean matches(RelNode rel) {
+ // Don't apply converters to converters that operate
+ // on the same RelTraitDef -- otherwise we get
+ // an n^2 effect.
+ if (rel instanceof Converter) {
+ if (((ConverterRule) getRule()).getTraitDef()
+ == ((Converter) rel).getTraitDef()) {
+ return false;
+ }
+ }
+ return super.matches(rel);
+ }
+ }
}
// End RelOptRule.java
http://git-wip-us.apache.org/repos/asf/calcite/blob/d81133fd/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperand.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperand.java b/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperand.java
index 12f52c1..d3c5f2f 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperand.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperand.java
@@ -69,37 +69,60 @@ public class RelOptRuleOperand {
* {@link RelOptRule#any},
* {@link RelOptRule#unordered},
* See {@link org.apache.calcite.plan.RelOptRuleOperandChildren} for more
- * details.</p>
+ * details.
*
* @param clazz Class of relational expression to match (must not be null)
* @param trait Trait to match, or null to match any trait
* @param predicate Predicate to apply to relational expression
* @param children Child operands
+ *
+ * @deprecated Use
+ * {@link RelOptRule#operand(Class, RelOptRuleOperandChildren)} or one of its
+ * overloaded methods.
*/
+ @Deprecated // to be removed before 2.0; see [CALCITE-1166]
protected <R extends RelNode> RelOptRuleOperand(
Class<R> clazz,
RelTrait trait,
Predicate<? super R> predicate,
RelOptRuleOperandChildren children) {
+ this(clazz, trait, predicate, children.policy, children.operands);
+ }
+
+ /** Private constructor.
+ *
+ * <p>Do not call from outside package, and do not create a sub-class.
+ *
+ * <p>The other constructor is deprecated; when it is removed, make fields
+ * {@link #parent}, {@link #ordinalInParent} and {@link #solveOrder} final,
+ * and add constructor parameters for them. See
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-1166">[CALCITE-1166]
+ * Disallow sub-classes of RelOptRuleOperand</a>. */
+ <R extends RelNode> RelOptRuleOperand(
+ Class<R> clazz,
+ RelTrait trait,
+ Predicate<? super R> predicate,
+ RelOptRuleOperandChildPolicy childPolicy,
+ ImmutableList<RelOptRuleOperand> children) {
assert clazz != null;
- switch (children.policy) {
+ switch (childPolicy) {
case ANY:
break;
case LEAF:
- assert children.operands.size() == 0;
+ assert children.size() == 0;
break;
case UNORDERED:
- assert children.operands.size() == 1;
+ assert children.size() == 1;
break;
default:
- assert children.operands.size() > 0;
+ assert children.size() > 0;
}
- this.childPolicy = children.policy;
+ this.childPolicy = childPolicy;
this.clazz = Preconditions.checkNotNull(clazz);
this.trait = trait;
//noinspection unchecked
this.predicate = Preconditions.checkNotNull((Predicate) predicate);
- this.children = children.operands;
+ this.children = children;
for (RelOptRuleOperand child : this.children) {
assert child.parent == null : "cannot re-use operands";
child.parent = this;
http://git-wip-us.apache.org/repos/asf/calcite/blob/d81133fd/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperandChildren.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperandChildren.java b/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperandChildren.java
index da007c6..9d7dc18 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperandChildren.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptRuleOperandChildren.java
@@ -18,6 +18,8 @@ package org.apache.calcite.plan;
import com.google.common.collect.ImmutableList;
+import java.util.List;
+
/**
* Children of a {@link org.apache.calcite.plan.RelOptRuleOperand} and the
* policy for matching them.
@@ -44,9 +46,9 @@ public class RelOptRuleOperandChildren {
public RelOptRuleOperandChildren(
RelOptRuleOperandChildPolicy policy,
- ImmutableList<RelOptRuleOperand> operands) {
+ List<RelOptRuleOperand> operands) {
this.policy = policy;
- this.operands = operands;
+ this.operands = ImmutableList.copyOf(operands);
}
}
http://git-wip-us.apache.org/repos/asf/calcite/blob/d81133fd/core/src/main/java/org/apache/calcite/rel/convert/ConverterRule.java
----------------------------------------------------------------------
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 5591ca3..7761488 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
@@ -19,7 +19,6 @@ package org.apache.calcite.rel.convert;
import org.apache.calcite.plan.Convention;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelOptRuleCall;
-import org.apache.calcite.plan.RelOptRuleOperand;
import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.plan.RelTraitDef;
import org.apache.calcite.rel.RelNode;
@@ -65,7 +64,7 @@ public abstract class ConverterRule extends RelOptRule {
public <R extends RelNode> ConverterRule(Class<R> clazz,
Predicate<? super R> predicate, RelTrait in, RelTrait out,
String description) {
- super(new ConverterRelOptRuleOperand(clazz, in, predicate),
+ super(convertOperand(clazz, predicate, in),
description == null
? "ConverterRule<in=" + in + ",out=" + out + ">"
: description);
@@ -122,28 +121,6 @@ public abstract class ConverterRule extends RelOptRule {
//~ Inner Classes ----------------------------------------------------------
- /**
- * Operand to an instance of the converter rule.
- */
- private static class ConverterRelOptRuleOperand extends RelOptRuleOperand {
- public <R extends RelNode> ConverterRelOptRuleOperand(Class<R> clazz,
- RelTrait in, Predicate<? super R> predicate) {
- super(clazz, in, predicate, any());
- }
-
- public boolean matches(RelNode rel) {
- // Don't apply converters to converters that operate
- // on the same RelTraitDef -- otherwise we get
- // an n^2 effect.
- if (rel instanceof Converter) {
- if (((ConverterRule) getRule()).getTraitDef()
- == ((Converter) rel).getTraitDef()) {
- return false;
- }
- }
- return super.matches(rel);
- }
- }
}
// End ConverterRule.java