FatLittle commented on a change in pull request #1991:
URL: https://github.com/apache/calcite/pull/1991#discussion_r436255565
##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
##########
@@ -317,8 +284,8 @@ RelSubset getOrCreateSubset(
subset.setDelivered();
}
- if (needsConverter && !planner.topDownOpt) {
- addConverters(subset, required, true);
+ if (needsConverter) {
+ addConverters(subset, required, !planner.topDownOpt);
Review comment:
Yes, it is.
The converters act as bridges between the working subset and other subsets.
Currently, the OptimizeGroup task will schedule tasks for all the logical Rels
in the RelSet while it skips physical Rels from other subsets. That's because
we are confident that if other subset can be reached by a converter, the
converter is already in the current working subset.
I was thinking of checking the upper bound before adding the enforcer. But I
cannot decide which place is suitable for adding the enforcer back when the
upper bound is changed. So I leave it for further optimizations.
----------------------------------------------------------------
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]
