hsyuan commented on a change in pull request #1772: [CALCITE-3744] Duplicate
rule matches when RelSet gets merged
URL: https://github.com/apache/calcite/pull/1772#discussion_r369271502
##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
##########
@@ -494,11 +494,10 @@ VolcanoRuleMatch popMatch(VolcanoPlannerPhase phase) {
}
}
- // 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(
Review comment:
You have to remove all the matches for the subset in `matchMap`, find them
in the `rulequeue` and remove them from `rulequeue`, and remove them from
`names` set. I don't think it is worth sweating.
Since the leak already exists before this change, I prefer leaving it as it
is.
I am going to propose a change to completely remove rulematch importance.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
