This is an automated email from the ASF dual-hosted git repository.
hyuan 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 77e7808 [CALCITE-3283] RelSubset does not contain its best RelNode
(Xiening Dai)
77e7808 is described below
commit 77e7808fc0aa4e089a992535bc890047724fd900
Author: Xiening Dai <[email protected]>
AuthorDate: Thu Aug 22 14:50:57 2019 -0700
[CALCITE-3283] RelSubset does not contain its best RelNode (Xiening Dai)
In VolcanoPlanner.rename(), the given relnode will be removed when we find
there's an equivalent rel after rename. But if the node to be removed
happens
to be the best relnode of its subset, the RelSubSet.best will not able to
get
updated under two scenarios:
1. If equivalent rel is in the same set as the original rel, currently we do
nothing. So RelSubSet.best is not updated and points to a node that's
removed
after rename.
2. If we end up merging two sub set and equivalent rel cost is same as
original
rel cost, we won't update RelSubSet.best either.
Fix the issue by udpdating the RelSubSet.best if best is the RelNode that's
supposed to be removed. Enable isValid() check only under debug log level or
finer until calcite-2018 is fixed.
Close #1403
---
.../java/org/apache/calcite/plan/volcano/VolcanoPlanner.java | 12 ++++++++++++
1 file changed, 12 insertions(+)
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 a49627a..43f79c4 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
@@ -882,6 +882,13 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
return litmus.fail("subset [{}] is in wrong set [{}]",
subset.getDescription(), set);
}
+
+ // Make sure best RelNode is valid
+ if (subset.best != null && !subset.set.rels.contains(subset.best)) {
+ return litmus.fail("RelSubset [{}] does not contain its best RelNode
[{}]",
+ subset.getDescription(), subset.best.getDescription());
+ }
+
for (RelNode rel : subset.getRels()) {
RelOptCost relCost = getCost(rel,
rel.getCluster().getMetadataQuery());
if (relCost.isLt(subset.bestCost)) {
@@ -1440,6 +1447,11 @@ public class VolcanoPlanner extends
AbstractRelOptPlanner {
boolean existed = subset.set.rels.remove(rel);
assert existed : "rel was not known to its set";
final RelSubset equivSubset = getSubset(equivRel);
+ if (subset.best == rel) {
+ subset.best = equivRel;
+ subset.bestCost = getCost(equivRel);
+ }
+
if (equivSubset != subset) {
// The equivalent relational expression is in a different
// subset, therefore the sets are equivalent.
