Skip to content

Commit

Permalink
Fix bug in HepPlanner which occurred when two RelNodes have identical…
Browse files Browse the repository at this point in the history
… digest but different row-type.
  • Loading branch information
danny0405 committed Aug 8, 2018
1 parent c845a1b commit 11ecf40
Showing 1 changed file with 13 additions and 7 deletions.
20 changes: 13 additions & 7 deletions core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.metadata.RelMetadataProvider;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.util.Pair;
import org.apache.calcite.util.Util;
import org.apache.calcite.util.graph.BreadthFirstIterator;
Expand Down Expand Up @@ -75,7 +76,7 @@ public class HepPlanner extends AbstractRelOptPlanner {

private RelTraitSet requestedRootTraits;

private Map<String, HepRelVertex> mapDigestToVertex;
private Map<Pair<String, RelDataType>, HepRelVertex> mapDigestToVertex;

private final Set<RelOptRule> allRules;

Expand Down Expand Up @@ -491,7 +492,8 @@ private Iterator<HepRelVertex> getGraphIterator(HepRelVertex start) {
/** Returns whether the vertex is valid. */
private boolean belongsToDag(HepRelVertex vertex) {
String digest = vertex.getCurrentRel().getDigest();
return mapDigestToVertex.get(digest) != null;
Pair<String, RelDataType> key = Pair.of(digest, vertex.getCurrentRel().getRowType());
return mapDigestToVertex.get(key) != null;
}

private HepRelVertex applyRule(
Expand Down Expand Up @@ -820,7 +822,8 @@ private HepRelVertex addRelToGraph(
if (!noDAG) {
// Now, check if an equivalent vertex already exists in graph.
String digest = rel.getDigest();
HepRelVertex equivVertex = mapDigestToVertex.get(digest);
Pair<String, RelDataType> key = Pair.of(digest, rel.getRowType());
HepRelVertex equivVertex = mapDigestToVertex.get(key);
if (equivVertex != null) {
// Use existing vertex.
return equivVertex;
Expand Down Expand Up @@ -887,18 +890,21 @@ private void updateVertex(HepRelVertex vertex, RelNode rel) {
notifyDiscard(vertex.getCurrentRel());
}
String oldDigest = vertex.getCurrentRel().toString();
if (mapDigestToVertex.get(oldDigest) == vertex) {
mapDigestToVertex.remove(oldDigest);
Pair<String, RelDataType> oldKey = Pair.of(oldDigest, vertex.getCurrentRel().getRowType());
if (mapDigestToVertex.get(oldKey) == vertex) {
mapDigestToVertex.remove(oldKey);
}
String newDigest = rel.getDigest();
Pair<String, RelDataType> newKey = Pair.of(newDigest, rel.getRowType());

// When a transformation happened in one rule apply, support
// vertex2 replace vertex1, but the current relNode of
// vertex1 and vertex2 is same,
// then the digest is also same. but we can't remove vertex2,
// otherwise the digest will be removed wrongly in the mapDigestToVertex
// when collectGC
// so it must update the digest that map to vertex
mapDigestToVertex.put(newDigest, vertex);
mapDigestToVertex.put(newKey, vertex);
if (rel != vertex.getCurrentRel()) {
vertex.replaceRel(rel);
}
Expand Down Expand Up @@ -963,7 +969,7 @@ private void collectGarbage() {
graphSizeLastGC = graph.vertexSet().size();

// Clean up digest map too.
Iterator<Map.Entry<String, HepRelVertex>> digestIter =
Iterator<Map.Entry<Pair<String, RelDataType>, HepRelVertex>> digestIter =
mapDigestToVertex.entrySet().iterator();
while (digestIter.hasNext()) {
HepRelVertex vertex = digestIter.next().getValue();
Expand Down

0 comments on commit 11ecf40

Please sign in to comment.