[CALCITE-3744] Duplicate rule matches when RelSet gets merged
Some times RelSet can get merged. e.g.
Each number represet Rel id, and they are in different RelSet.
1 4
\ /
2 /
\ /
3
Assume in the rulequeue, we have rule match with name of 12, 23, 43. For
simplicity, omit the rule name, first digit represent the parent RelNode,
second represent the child.
If after some rule, we merged the set of 3 into the set of 2, and RelNode 4's
input is replaced by RelSubset of 2. We will retrigger rules and try to add
rulematch 12, 42, 43 into rule queue.
12 will be filtered out, because there is duplicate in the RuleMatch names set.
But for 43, it now has different rulematch digest with previous match, because
RelNode #4's input RelSubset changed, its digest also changed. So we can't
detect the duplication and will re-apply rulematch 43 twice.
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java b/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
index dca327e..7e851f8 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
@@ -494,11 +494,10 @@
}
}
- // A rule match's digest is composed of the operand RelNodes' digests,
- // which may have changed if sets have merged since the rule match was
- // enqueued.
- match.recomputeDigest();
-
+ // If sets have merged since the rule match was enqueued, the match
+ // may not be removed from the matchMap because the subset may have
+ // changed, it is OK to leave it since the matchMap will be cleared
+ // at the end.
phaseMatchList.matchMap.remove(
planner.getSubset(match.rels[0]), match);
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java
index 12e7831..9c3e9d3 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleMatch.java
@@ -17,7 +17,6 @@
package org.apache.calcite.plan.volcano;
import org.apache.calcite.plan.RelOptRuleOperand;
-import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.plan.RelTraitSet;
import org.apache.calcite.rel.RelNode;
@@ -143,18 +142,18 @@
new StringBuilder("rule [" + getRule() + "] rels [");
for (int i = 0; i < rels.length; i++) {
if (i > 0) {
- buf.append(", ");
+ buf.append(',');
}
- RelOptUtil.appendRelDescription(buf, rels[i]);
+ buf.append('#').append(rels[i].getId());
}
- buf.append("]");
+ buf.append(']');
return buf.toString();
}
/**
- * Recomputes the digest of this VolcanoRuleMatch. It is necessary when sets
- * have merged since the match was created.
+ * Recomputes the digest of this VolcanoRuleMatch.
*/
+ @Deprecated // to be removed before 2.0
public void recomputeDigest() {
digest = computeDigest();
}
