hsyuan commented on a change in pull request #1949:
URL: https://github.com/apache/calcite/pull/1949#discussion_r415797305



##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
##########
@@ -361,26 +361,16 @@ void mergeWith(
 
     Set<RelNode> parentRels = new HashSet<>(parents);
     for (RelNode otherRel : otherSet.rels) {
-      if (planner.prunedNodes.contains(otherRel)) {
-        continue;
-      }
-
-      boolean pruned = false;
       if (parentRels.contains(otherRel)) {

Review comment:
       Can you also add a check `otherRel` is not `spool` and is not an 
enforcer before `parentRels.contains(otherRel)`? I think we can't prune them in 
that case, what do you think?

##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
##########
@@ -361,26 +361,16 @@ void mergeWith(
 
     Set<RelNode> parentRels = new HashSet<>(parents);
     for (RelNode otherRel : otherSet.rels) {
-      if (planner.prunedNodes.contains(otherRel)) {
-        continue;
-      }
-
-      boolean pruned = false;
       if (parentRels.contains(otherRel)) {

Review comment:
       Maybe we can also remove 
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java#L127-L130,
 they look duplicate with this part.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to