mihaibudiu commented on code in PR #3851:
URL: https://github.com/apache/calcite/pull/3851#discussion_r1678640256
##########
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java:
##########
@@ -982,11 +982,25 @@ void propagateCostImprovements(RelNode rel) {
if (!relNode.getTraitSet().satisfies(subset.getTraitSet())) {
continue;
}
- if (!cost.isLt(subset.bestCost)) {
+
+ // Update subset best and best's cost when we find a cheaper rel
+ if (relNode != subset.best && !cost.isLt(subset.bestCost)) {
continue;
}
- // Update subset best cost when we find a cheaper rel or the current
- // best's cost is changed
+
+ // Update the RelNode's cost when we find that the cost is changed.
+ //
+ // Why do we need to update it?
+ // When RelSet's one of the subsets find a cheaper RelNode, we need to
update the
+ // parents of the subset to have the best RelNode and best cost.
+ // In theory, this cost will become smaller, But according to
+ // The SQL we added in the JdbcAdapterTest {@link
testVolcanoPlannerInternalValid},
+ // it shows RelNode's cost will become bigger sometimes.
+ // So we update it.
Review Comment:
Yes, I think comments should be impersonal - if you want to say "I" you
should probably sign it in the code as well.
The comment could be just `cost X can be bigger than cost Y, as shown by the
test @link ...`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
