zhuwenzhuang commented on code in PR #4803:
URL: https://github.com/apache/calcite/pull/4803#discussion_r2958513522
##########
core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java:
##########
@@ -136,11 +139,13 @@ public class HepPlanner extends AbstractRelOptPlanner {
*
* <p>Value: match-key ID lists in {@link #firedRulesCache} that contain the
key ID.
*/
- private final Multimap<Integer, List<Integer>> firedRulesCacheIndex =
HashMultimap.create();
-
+ private final Multimap<Integer, ImmutableIntList> firedRulesCacheIndex =
HashMultimap.create();
private boolean enableFiredRulesCache = false;
+ private boolean largePlanMode = false;
Review Comment:
1) LargePlanBenchmark includes a test compare different match orders’ rule
match count. it is a specific test to verify the correctness.
2) I think this also depends on how the community views the specific
compatibility impact of this change. The optimization still guarantees that
planning does not stop until no more rules can be matched.
However, if a user has some hacky rules that rely on the traversal order
strictly following a fixed pattern, the optimization result may change for
those users. I believe such users are actually using HepPlanner incorrectly. If
everyone thinks the risk of enabling this by default is acceptable, I would
prefer to turn this optimization on by default.
--
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]
