morrySnow commented on code in PR #35925:
URL: https://github.com/apache/doris/pull/35925#discussion_r1650951923
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java:
##########
@@ -70,12 +75,46 @@ public void execute(JobContext jobContext) {
rewriteJobs, currentCtx.getRewritePlan());
return;
}
+ Pair<Boolean, Boolean> checkHint = checkRuleHint();
+ if (checkHint.first) {
+ if (checkHint.second) {
+ currentCtx.setRewritePlan(applyCboRuleCtx.getRewritePlan());
+ }
+ return;
+ }
// If the candidate applied cbo rule is better, replace the original
plan with it.
if (appliedCboRuleCost.get().first.getValue() <
skipCboRuleCost.get().first.getValue()) {
currentCtx.setRewritePlan(applyCboRuleCtx.getRewritePlan());
}
}
+ private Pair<Boolean, Boolean> checkRuleHint() {
Review Comment:
add comment to explain the first and second of the return Pair
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java:
##########
@@ -70,12 +75,46 @@ public void execute(JobContext jobContext) {
rewriteJobs, currentCtx.getRewritePlan());
return;
}
+ Pair<Boolean, Boolean> checkHint = checkRuleHint();
+ if (checkHint.first) {
+ if (checkHint.second) {
+ currentCtx.setRewritePlan(applyCboRuleCtx.getRewritePlan());
+ }
+ return;
+ }
// If the candidate applied cbo rule is better, replace the original
plan with it.
if (appliedCboRuleCost.get().first.getValue() <
skipCboRuleCost.get().first.getValue()) {
currentCtx.setRewritePlan(applyCboRuleCtx.getRewritePlan());
}
}
+ private Pair<Boolean, Boolean> checkRuleHint() {
+ Pair<Boolean, Boolean> checkResult = Pair.of(false, false);
+ if (rewriteJobs.get(0) instanceof RootPlanTreeRewriteJob) {
+ for (Rule rule : ((RootPlanTreeRewriteJob)
rewriteJobs.get(0)).getRules()) {
+ checkResult = checkRuleHintWithHintName(rule.getRuleType());
+ }
+ }
+ if (rewriteJobs.get(0) instanceof CustomRewriteJob) {
+ checkResult = checkRuleHintWithHintName(((CustomRewriteJob)
rewriteJobs.get(0)).getRuleType());
+ }
+ return checkResult;
+ }
+
+ private Pair<Boolean, Boolean> checkRuleHintWithHintName(RuleType
ruleType) {
Review Comment:
add comment to explain the first and second of the return Pair
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownAggThroughJoinOneSide.java:
##########
@@ -80,7 +80,8 @@ public List<Rule> buildRules() {
.thenApply(ctx -> {
Set<Integer> enableNereidsRules =
ctx.cascadesContext.getConnectContext()
.getSessionVariable().getEnableNereidsRules();
- if
(!enableNereidsRules.contains(RuleType.PUSH_DOWN_AGG_THROUGH_JOIN_ONE_SIDE.type()))
{
+ if
(!enableNereidsRules.contains(RuleType.PUSH_DOWN_AGG_THROUGH_JOIN_ONE_SIDE.type())
+ &&
!RuleType.PUSH_DOWN_AGG_THROUGH_JOIN_ONE_SIDE.checkUseCboRuleHint()) {
Review Comment:
why add here? i think the mod of cost based rewrite is enough
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java:
##########
@@ -70,12 +75,46 @@ public void execute(JobContext jobContext) {
rewriteJobs, currentCtx.getRewritePlan());
return;
}
+ Pair<Boolean, Boolean> checkHint = checkRuleHint();
+ if (checkHint.first) {
Review Comment:
if no need to rewrite, return at the beginning of execute to avoid useless
rewrite for better perf
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
