morrySnow commented on code in PR #59517:
URL: https://github.com/apache/doris/pull/59517#discussion_r2659501716
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java:
##########
@@ -56,6 +61,11 @@ public CostBasedRewriteJob(List<RewriteJob> rewriteJobs) {
// need to generate real rewrite job list
}
+ private void restoreCteProducerMap(StatementContext context, Map<CTEId,
LogicalPlan> currentCteProducers) {
+ context.getRewrittenCteProducer().clear();
+ currentCteProducers.forEach(context.getRewrittenCteProducer()::put);
Review Comment:
why not put all?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java:
##########
@@ -94,8 +111,7 @@ public void execute(JobContext jobContext) {
}
// If the candidate applied cbo rule is better, replace the original
plan with it.
if (appliedCboRuleCost.get().first.getValue() <
skipCboRuleCost.get().first.getValue()) {
- currentCtx.addPlanProcesses(applyCboRuleCtx.getPlanProcesses());
Review Comment:
why remove `addPlanProcesses`?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java:
##########
@@ -69,14 +79,21 @@ public void execute(JobContext jobContext) {
CascadesContext applyCboRuleCtx =
CascadesContext.newCurrentTreeContext(currentCtx);
// execute cbo rule on one candidate
Rewriter.getCteChildrenRewriter(applyCboRuleCtx,
rewriteJobs).execute();
+ Plan applyCboPlan = applyCboRuleCtx.getRewritePlan();
if
(skipCboRuleCtx.getRewritePlan().deepEquals(applyCboRuleCtx.getRewritePlan())) {
// this means rewrite do not do anything
return;
}
+ Map<CTEId, LogicalPlan> currentCteProducers = Maps.newHashMap();
+ // cost based rewrite job may contaminate
StatementContext.rewrittenCteProducer
+ // clone current rewrittenCteProducer, and restore it after getCost(.).
+
currentCtx.getStatementContext().getRewrittenCteProducer().forEach(currentCteProducers::put);
Review Comment:
only process `RewrittenCteProducer` is enough? maybe we should process all
cte related context just like what we do in `ClearContextStatus`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
