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 2020/02/22 06:38:02 UTC
[calcite] branch master updated: Following CALCITE-3812: move
HintStrategy out of HintStrategyTable
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
The following commit(s) were added to refs/heads/master by this push:
new 7c27b14 Following CALCITE-3812: move HintStrategy out of HintStrategyTable
7c27b14 is described below
commit 7c27b147414c64505fa33c947100ece094caa15c
Author: yuzhao.cyz <yu...@gmail.com>
AuthorDate: Sat Feb 22 09:56:49 2020 +0800
Following CALCITE-3812: move HintStrategy out of HintStrategyTable
Also delete ExplicitHintPredicate because it's useless, implement the
HintPredicate directly instead.
---
.../calcite/rel/hint/ExplicitHintMatcher.java | 47 -------
.../calcite/rel/hint/ExplicitHintPredicate.java | 49 -------
.../org/apache/calcite/rel/hint/HintPredicate.java | 7 +
.../apache/calcite/rel/hint/HintPredicates.java | 10 --
.../org/apache/calcite/rel/hint/HintStrategy.java | 151 +++++++++++++++++++++
.../apache/calcite/rel/hint/HintStrategyTable.java | 125 +----------------
.../org/apache/calcite/rel/hint/package-info.java | 17 +--
.../apache/calcite/test/SqlHintsConverterTest.java | 9 +-
8 files changed, 171 insertions(+), 244 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/ExplicitHintMatcher.java b/core/src/main/java/org/apache/calcite/rel/hint/ExplicitHintMatcher.java
deleted file mode 100644
index 647d742..0000000
--- a/core/src/main/java/org/apache/calcite/rel/hint/ExplicitHintMatcher.java
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to you under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.calcite.rel.hint;
-
-import org.apache.calcite.rel.RelNode;
-
-/**
- * A function to customize whether a relational expression should match a hint.
- *
- * <p>Usually you may not need to implement this function,
- * {@link NodeTypeHintPredicate} is enough for most of the {@link RelHint}s.
- *
- * <p>Some of the hints can only be matched to the relational
- * expression with special match conditions(not only the relational expression type).
- * i.e. "hash_join(r, st)", this hint can only be applied to JOIN expression that
- * has "r" and "st" as the input table names. To implement this, you can customize an
- * {@link ExplicitHintPredicate} with the {@link ExplicitHintMatcher}.
- *
- * @see ExplicitHintPredicate
- * @see HintPredicates
- */
-@FunctionalInterface
-public interface ExplicitHintMatcher {
-
- /**
- * Returns whether the given hint can attach to the relational expression.
- *
- * @param hint Hints
- * @param node Relational expression to test if the hint matches
- * @return true if the {@code hint} can attach to the {@code node}
- */
- boolean matches(RelHint hint, RelNode node);
-}
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/ExplicitHintPredicate.java b/core/src/main/java/org/apache/calcite/rel/hint/ExplicitHintPredicate.java
deleted file mode 100644
index 59b2d04..0000000
--- a/core/src/main/java/org/apache/calcite/rel/hint/ExplicitHintPredicate.java
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to you under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.calcite.rel.hint;
-
-import org.apache.calcite.rel.RelNode;
-
-/**
- * A hint predicate with custom test rules.
- *
- * @see ExplicitHintMatcher
- */
-public class ExplicitHintPredicate implements HintPredicate {
- //~ Instance fields --------------------------------------------------------
-
- private final ExplicitHintMatcher matcher;
-
- /**
- * Creates an {@link ExplicitHintPredicate} with specified {@code matcher}.
- *
- * <p>Make this constructor package-protected intentionally, use
- * {@link HintPredicates#explicit(ExplicitHintMatcher)}.
- *
- * @param matcher ExplicitHintMatcher instance to test
- * if a hint can be applied to a rel
- */
- ExplicitHintPredicate(ExplicitHintMatcher matcher) {
- this.matcher = matcher;
- }
-
- //~ Methods ----------------------------------------------------------------
-
- @Override public boolean apply(RelHint hint, RelNode rel) {
- return this.matcher.matches(hint, rel);
- }
-}
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/HintPredicate.java b/core/src/main/java/org/apache/calcite/rel/hint/HintPredicate.java
index 1c0c102..0c52b3e 100644
--- a/core/src/main/java/org/apache/calcite/rel/hint/HintPredicate.java
+++ b/core/src/main/java/org/apache/calcite/rel/hint/HintPredicate.java
@@ -27,6 +27,13 @@ import org.apache.calcite.rel.RelNode;
* that this hint would be propagated and applied to the {@link org.apache.calcite.rel.core.Join}
* relational expressions.
*
+ * <p>Usually use {@link NodeTypeHintPredicate} is enough for most of the {@link RelHint}s.
+ * Some of the hints can only be matched to the relational expression with special
+ * match conditions(not only the relational expression type).
+ * i.e. "hash_join(r, st)", this hint can only be applied to JOIN expression that
+ * has "r" and "st" as the input table names. To implement this, you can make a custom
+ * {@code HintPredicate} instance.
+ *
* <p>A {@code HintPredicate} can be used independently or cascaded with other strategies
* with method {@link HintPredicates#and}.
*
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/HintPredicates.java b/core/src/main/java/org/apache/calcite/rel/hint/HintPredicates.java
index da1f90b..266ccf4 100644
--- a/core/src/main/java/org/apache/calcite/rel/hint/HintPredicates.java
+++ b/core/src/main/java/org/apache/calcite/rel/hint/HintPredicates.java
@@ -51,16 +51,6 @@ public abstract class HintPredicates {
new NodeTypeHintPredicate(NodeTypeHintPredicate.NodeType.CALC);
/**
- * Creates a hint predicate from a specific matcher whose rules are totally customized.
- *
- * @param matcher The hint matcher
- * @return A ExplicitHintPredicate instance
- */
- public static HintPredicate explicit(ExplicitHintMatcher matcher) {
- return new ExplicitHintPredicate(matcher);
- }
-
- /**
* Returns a composed hint predicate that represents a short-circuiting logical
* AND of an array of hint predicates {@code hintPredicates}. When evaluating the composed
* predicate, if a predicate is {@code false}, then all the left
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/HintStrategy.java b/core/src/main/java/org/apache/calcite/rel/hint/HintStrategy.java
new file mode 100644
index 0000000..c58a58c
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/rel/hint/HintStrategy.java
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.hint;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.rel.convert.ConverterRule;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+/**
+ * Represents a hint strategy entry of {@link HintStrategyTable}.
+ *
+ * <p>A {@code HintStrategy} includes:
+ *
+ * <ul>
+ * <li>{@link HintPredicate}: tests whether a hint should apply to
+ * a relational expression;</li>
+ * <li>{@link HintOptionChecker}: validates the hint options;</li>
+ * <li>{@code excludedRules}: rules to exclude when a relational expression
+ * is going to apply a planner rule;</li>
+ * <li>{@code converterRules}: fallback rules to apply when there are
+ * no proper implementations after excluding the {@code excludedRules}.</li>
+ * </ul>
+ *
+ * <p>The {@link HintPredicate} is required, all the other items are optional.
+ *
+ * <p>A {@link HintStrategy} is immutable.
+ */
+public class HintStrategy {
+ //~ Instance fields --------------------------------------------------------
+
+ public final HintPredicate predicate;
+ public final HintOptionChecker hintOptionChecker;
+ public final ImmutableSet<RelOptRule> excludedRules;
+ public final ImmutableSet<ConverterRule> converterRules;
+
+ //~ Constructors -----------------------------------------------------------
+
+ private HintStrategy(
+ HintPredicate predicate,
+ HintOptionChecker hintOptionChecker,
+ ImmutableSet<RelOptRule> excludedRules,
+ ImmutableSet<ConverterRule> converterRules) {
+ this.predicate = predicate;
+ this.hintOptionChecker = hintOptionChecker;
+ this.excludedRules = excludedRules;
+ this.converterRules = converterRules;
+ }
+
+ /**
+ * Returns a {@link HintStrategy} builder with given hint predicate.
+ *
+ * @param hintPredicate hint predicate
+ * @return {@link Builder} instance
+ */
+ public static Builder builder(HintPredicate hintPredicate) {
+ return new Builder(hintPredicate);
+ }
+
+ //~ Inner Class ------------------------------------------------------------
+
+ /** Builder for {@link HintStrategy}. */
+ public static class Builder {
+ private final HintPredicate predicate;
+ @Nullable
+ private HintOptionChecker optionChecker;
+ private ImmutableSet<RelOptRule> excludedRules;
+ private ImmutableSet<ConverterRule> converterRules;
+
+ private Builder(HintPredicate predicate) {
+ this.predicate = Objects.requireNonNull(predicate);
+ this.excludedRules = ImmutableSet.of();
+ this.converterRules = ImmutableSet.of();
+ }
+
+ /** Registers a hint option checker to validate the hint options. */
+ public Builder optionChecker(HintOptionChecker optionChecker) {
+ this.optionChecker = Objects.requireNonNull(optionChecker);
+ return this;
+ }
+
+ /**
+ * Registers an array of rules to exclude during the
+ * {@link org.apache.calcite.plan.RelOptPlanner} planning.
+ *
+ * <p>The desired converter rules work together with the excluded rules.
+ * We have no validation here but they expect to have the same
+ * function(semantic equivalent).
+ *
+ * <p>A rule fire cancels if:
+ *
+ * <ol>
+ * <li>The registered {@link #excludedRules} contains the rule</li>
+ * <li>And the desired converter rules conversion is not possible
+ * for the rule matched root node</li>
+ * </ol>
+ *
+ * @param rules excluded rules
+ */
+ public Builder excludedRules(RelOptRule... rules) {
+ this.excludedRules = ImmutableSet.copyOf(rules);
+ return this;
+ }
+
+ /**
+ * Registers an array of desired converter rules during the
+ * {@link org.apache.calcite.plan.RelOptPlanner} planning.
+ *
+ * <p>The desired converter rules work together with the excluded rules.
+ * We have no validation here but they expect to have the same
+ * function(semantic equivalent).
+ *
+ * <p>A rule fire cancels if:
+ *
+ * <ol>
+ * <li>The registered {@link #excludedRules} contains the rule</li>
+ * <li>And the desired converter rules conversion is not possible
+ * for the rule matched root node</li>
+ * </ol>
+ *
+ * <p>If no converter rules are specified, we assume the conversion is possible.
+ *
+ * @param rules desired converter rules
+ */
+ public Builder converterRules(ConverterRule... rules) {
+ this.converterRules = ImmutableSet.copyOf(rules);
+ return this;
+ }
+
+ public HintStrategy build() {
+ return new HintStrategy(predicate, optionChecker, excludedRules, converterRules);
+ }
+ }
+}
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/HintStrategyTable.java b/core/src/main/java/org/apache/calcite/rel/hint/HintStrategyTable.java
index 07f1949..ac75153 100644
--- a/core/src/main/java/org/apache/calcite/rel/hint/HintStrategyTable.java
+++ b/core/src/main/java/org/apache/calcite/rel/hint/HintStrategyTable.java
@@ -22,8 +22,6 @@ import org.apache.calcite.rel.convert.ConverterRule;
import org.apache.calcite.util.Litmus;
import org.apache.calcite.util.trace.CalciteTrace;
-import com.google.common.collect.ImmutableSet;
-
import org.slf4j.Logger;
import java.util.Collections;
@@ -34,7 +32,6 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
-import javax.annotation.Nullable;
/**
* A collections of {@link HintStrategy}s.
@@ -44,7 +41,7 @@ import javax.annotation.Nullable;
* would be propagated and attached to the {@link org.apache.calcite.rel.core.Join}
* relational expressions.
*
- * <p>The matching for hint name is case in-sensitive.
+ * <p>Match of hint name is case in-sensitive.
*
* @see HintPredicate
*/
@@ -151,16 +148,6 @@ public class HintStrategyTable {
return new Builder();
}
- /**
- * Returns a {@link HintStrategy} builder with given hint predicate.
- *
- * @param hintPredicate hint predicate
- * @return {@link StrategyBuilder} instance
- */
- public static StrategyBuilder strategyBuilder(HintPredicate hintPredicate) {
- return new StrategyBuilder(hintPredicate);
- }
-
//~ Inner Class ------------------------------------------------------------
/**
@@ -202,7 +189,7 @@ public class HintStrategyTable {
public Builder hintStrategy(String hintName, HintPredicate strategy) {
this.strategies.put(Key.of(hintName),
- strategyBuilder(Objects.requireNonNull(strategy)).build());
+ HintStrategy.builder(Objects.requireNonNull(strategy)).build());
return this;
}
@@ -230,114 +217,6 @@ public class HintStrategyTable {
}
}
- /**
- * Represents a hint strategy entry of this {@link HintStrategyTable}.
- *
- * <p>A {@code HintStrategy} includes:
- *
- * <ul>
- * <li>{@link HintPredicate}: tests whether a hint should apply to
- * a relational expression;</li>
- * <li>{@link HintOptionChecker}: validates the hint options;</li>
- * <li>{@code excludedRules}: rules to exclude when a relational expression
- * is going to apply a planner rule;</li>
- * <li>{@code converterRules}: fallback rules to apply when there are
- * no proper implementations after excluding the {@code excludedRules}.</li>
- * </ul>
- *
- * <p>The {@link HintPredicate} is required, all the other items are optional.
- */
- public static class HintStrategy {
- public final HintPredicate predicate;
- public final HintOptionChecker hintOptionChecker;
- public final ImmutableSet<RelOptRule> excludedRules;
- public final ImmutableSet<ConverterRule> converterRules;
-
- public HintStrategy(
- HintPredicate predicate,
- HintOptionChecker hintOptionChecker,
- ImmutableSet<RelOptRule> excludedRules,
- ImmutableSet<ConverterRule> converterRules) {
- this.predicate = predicate;
- this.hintOptionChecker = hintOptionChecker;
- this.excludedRules = excludedRules;
- this.converterRules = converterRules;
- }
- }
-
- /** Builder for {@link HintStrategy}. */
- public static class StrategyBuilder {
- private final HintPredicate predicate;
- @Nullable
- private HintOptionChecker optionChecker;
- private ImmutableSet<RelOptRule> excludedRules;
- private ImmutableSet<ConverterRule> converterRules;
-
- private StrategyBuilder(HintPredicate predicate) {
- this.predicate = Objects.requireNonNull(predicate);
- this.excludedRules = ImmutableSet.of();
- this.converterRules = ImmutableSet.of();
- }
-
- /** Registers a hint option checker to validate the hint options. */
- public StrategyBuilder optionChecker(HintOptionChecker optionChecker) {
- this.optionChecker = Objects.requireNonNull(optionChecker);
- return this;
- }
-
- /**
- * Registers an array of rules to exclude during the
- * {@link org.apache.calcite.plan.RelOptPlanner} planning.
- *
- * <p>The desired converter rules work together with the excluded rules.
- * We have no validation here but they expect to have the same
- * function(semantic equivalent).
- *
- * <p>A rule fire cancels if:
- *
- * <ol>
- * <li>The registered {@link #excludedRules} contains the rule</li>
- * <li>The desired converter rules conversion is not possible for the rule
- * matched root node</li>
- * </ol>
- *
- * @param rules excluded rules
- */
- public StrategyBuilder excludedRules(RelOptRule... rules) {
- this.excludedRules = ImmutableSet.copyOf(rules);
- return this;
- }
-
- /**
- * Registers an array of desired converter rules during the
- * {@link org.apache.calcite.plan.RelOptPlanner} planning.
- *
- * <p>The desired converter rules work together with the excluded rules.
- * We have no validation here but they expect to have the same
- * function(semantic equivalent).
- *
- * <p>A rule fire cancels if:
- *
- * <ol>
- * <li>The registered {@link #excludedRules} contains the rule</li>
- * <li>The desired converter rules conversion is not possible for the rule
- * matched root node</li>
- * </ol>
- *
- * <p>If no converter rules are specified, we assume the conversion is possible.
- *
- * @param rules desired converter rules
- */
- public StrategyBuilder converterRules(ConverterRule... rules) {
- this.converterRules = ImmutableSet.copyOf(rules);
- return this;
- }
-
- public HintStrategy build() {
- return new HintStrategy(predicate, optionChecker, excludedRules, converterRules);
- }
- }
-
/** Implementation of {@link org.apache.calcite.util.Litmus} that returns
* a status code, it logs warnings for fail check and does not throw. */
public static class HintErrorLogger implements Litmus {
diff --git a/core/src/main/java/org/apache/calcite/rel/hint/package-info.java b/core/src/main/java/org/apache/calcite/rel/hint/package-info.java
index b8522e6e..25c28c1 100644
--- a/core/src/main/java/org/apache/calcite/rel/hint/package-info.java
+++ b/core/src/main/java/org/apache/calcite/rel/hint/package-info.java
@@ -31,20 +31,15 @@
* on emp.deptno=dept.deptno
* </pre></blockquote>
*
- * <h2>Customize Hint Matching Rules</h2>
+ * <h2>Customize Hint Match Rules</h2>
* Calcite implements a framework to define and propagate the hints. In order to make the hints
* propagate efficiently, every hint referenced in the sql statement needs to
- * register the matching rules for hints propagation.
+ * register the match rules for hints propagation.
*
- * <p>A matching rule is defined though {@link org.apache.calcite.rel.hint.HintPredicate}.
- * Two kinds of matching rules are supported for hint registration:
- *
- * <ol>
- * <li>{@link org.apache.calcite.rel.hint.NodeTypeHintPredicate} matches a
- * relational expression by the relational node type.</li>
- * <li>{@link org.apache.calcite.rel.hint.ExplicitHintPredicate} matches a
- * relational expression with totally customized matching rule.</li>
- * </ol>
+ * <p>A match rule is defined though {@link org.apache.calcite.rel.hint.HintPredicate}.
+ * {@link org.apache.calcite.rel.hint.NodeTypeHintPredicate} matches a relational expression
+ * by its node type; you can also define a custom instance with more complicated rules,
+ * i.e. JOIN with specified relations from the hint options.
*
* <p>Here is the code snippet to illustrate how to config the strategies:
*
diff --git a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
index 90c87d8..d8c1284 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
@@ -42,6 +42,7 @@ import org.apache.calcite.rel.core.JoinInfo;
import org.apache.calcite.rel.core.TableScan;
import org.apache.calcite.rel.hint.HintPredicate;
import org.apache.calcite.rel.hint.HintPredicates;
+import org.apache.calcite.rel.hint.HintStrategy;
import org.apache.calcite.rel.hint.HintStrategyTable;
import org.apache.calcite.rel.hint.Hintable;
import org.apache.calcite.rel.hint.RelHint;
@@ -753,7 +754,7 @@ public class SqlHintsConverterTest extends SqlToRelTestBase {
"resource", HintPredicates.or(
HintPredicates.PROJECT, HintPredicates.AGGREGATE, HintPredicates.CALC))
.hintStrategy("AGG_STRATEGY",
- HintStrategyTable.strategyBuilder(HintPredicates.AGGREGATE)
+ HintStrategy.builder(HintPredicates.AGGREGATE)
.optionChecker(
(hint, errorHandler) -> errorHandler.check(
hint.listOptions.size() == 1
@@ -765,7 +766,7 @@ public class SqlHintsConverterTest extends SqlToRelTestBase {
.hintStrategy("use_hash_join",
HintPredicates.and(HintPredicates.JOIN, joinWithFixedTableName()))
.hintStrategy("use_merge_join",
- HintStrategyTable.strategyBuilder(
+ HintStrategy.builder(
HintPredicates.and(HintPredicates.JOIN, joinWithFixedTableName()))
.excludedRules(EnumerableRules.ENUMERABLE_JOIN_RULE).build())
.build();
@@ -773,7 +774,7 @@ public class SqlHintsConverterTest extends SqlToRelTestBase {
/** Returns a {@link HintPredicate} for join with specified table references. */
private static HintPredicate joinWithFixedTableName() {
- return HintPredicates.explicit((hint, rel) -> {
+ return (hint, rel) -> {
if (!(rel instanceof LogicalJoin)) {
return false;
}
@@ -784,7 +785,7 @@ public class SqlHintsConverterTest extends SqlToRelTestBase {
.map(scan -> Util.last(scan.getTable().getQualifiedName()))
.collect(Collectors.toList());
return equalsStringList(tableNames, inputTables);
- });
+ };
}
/** Format the query with hint {@link #HINT}. */