mihaibudiu commented on code in PR #3860:
URL: https://github.com/apache/calcite/pull/3860#discussion_r1679687994
##########
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java:
##########
@@ -988,15 +988,15 @@ void propagateCostImprovements(RelNode rel) {
continue;
}
- // 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.
+ // The cost of the RelNode is updated when a change is detected.
Review Comment:
This looks good. If the original author can confirm that this is that the
rewrites convey his intention, we can merge it.
I am not sure about the commit message, because I don't know what "strong"
is.
--
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]