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 e1e5898 [CALCITE-3807] checkForSatisfiedConverters() is unnecessary
e1e5898 is described below
commit e1e5898f8975f2478862973c19799c8be45b8d83
Author: Xiening Dai <[email protected]>
AuthorDate: Wed Feb 19 15:09:02 2020 -0800
[CALCITE-3807] checkForSatisfiedConverters() is unnecessary
When VolcanoPlanner registers an abstract converter, it adds the converter
into set.abstractConverters list, then calls checkSatisfiedConverter() to see
if any converter is satisfied and can be remove from the list. But for every
abstract converter, it always satisfies itself (changeTraitsUsingConverters()
returns itself). Basically the converter would be removed from the list right
after it's added. So this check is completely unnecessary and it slows down the
planner.
---
.../org/apache/calcite/plan/volcano/RelSet.java | 6 -----
.../org/apache/calcite/plan/volcano/RelSubset.java | 1 -
.../calcite/plan/volcano/VolcanoPlanner.java | 29 ----------------------
3 files changed, 36 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
b/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
index 10816cc..974e331 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
@@ -63,12 +63,6 @@ class RelSet {
final List<RelSubset> subsets = new ArrayList<>();
/**
- * List of {@link AbstractConverter} objects which have not yet been
- * satisfied.
- */
- final List<AbstractConverter> abstractConverters = new ArrayList<>();
-
- /**
* Set to the superseding set when this is found to be equivalent to another
* set.
*/
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
index 13bd1f9..22fcf14 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
@@ -380,7 +380,6 @@ public class RelSubset extends AbstractRelNode {
}
}
}
- planner.checkForSatisfiedConverters(set, rel);
}
} finally {
activeSet.remove(this);
diff --git
a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
index 5b5df67..a858aa0 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
@@ -1068,27 +1068,6 @@ public class VolcanoPlanner extends
AbstractRelOptPlanner {
return converted;
}
- void checkForSatisfiedConverters(
- RelSet set,
- RelNode rel) {
- int i = 0;
- while (i < set.abstractConverters.size()) {
- AbstractConverter converter = set.abstractConverters.get(i);
- RelNode converted =
- changeTraitsUsingConverters(
- rel,
- converter.getTraitSet());
- if (converted == null) {
- i++; // couldn't convert this; move on to the next
- } else {
- if (!isRegistered(converted)) {
- registerImpl(converted, set);
- }
- set.abstractConverters.remove(converter); // success
- }
- }
- }
-
public void setImportance(RelNode rel, double importance) {
assert rel != null;
if (importance == 0d) {
@@ -1756,14 +1735,6 @@ public class VolcanoPlanner extends
AbstractRelOptPlanner {
ruleQueue.subsetImportances.remove(subset);
}
- // Remember abstract converters until they're satisfied
- if (rel instanceof AbstractConverter) {
- set.abstractConverters.add((AbstractConverter) rel);
- }
-
- // If this set has any unsatisfied converters, try to satisfy them.
- checkForSatisfiedConverters(set, rel);
-
// Make sure this rel's subset importance is updated
ruleQueue.recompute(subset, true);
