[fix](CostBasedRewriteJob)restore StatementContext.rewrittenCteProducer in CostBasedRewriteJob #59517
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
englefly
force-pushed
the
cte-producer-costrewrite
branch
from
6e97d27 to
073c151
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 32774 ms |
TPC-DS: Total hot run time: 173200 ms |
ClickBench: Total hot run time: 26.79 s |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 32270 ms |
TPC-DS: Total hot run time: 172742 ms |
ClickBench: Total hot run time: 27.06 s |
englefly
force-pushed
the
cte-producer-costrewrite
branch
from
ef065c6 to
4438cb1
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 32582 ms |
TPC-DS: Total hot run time: 173742 ms |
ClickBench: Total hot run time: 27.28 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
morrySnow
reviewed
Jan 4, 2026
|
|
||
| private void restoreCteProducerMap(StatementContext context, Map<CTEId, LogicalPlan> currentCteProducers) { | ||
| context.getRewrittenCteProducer().clear(); | ||
| currentCteProducers.forEach(context.getRewrittenCteProducer()::put); |
Contributor
Author
There was a problem hiding this comment.
你的问题是 为什么不put all吗？这里就是put all
| } | ||
| // 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()); |
Contributor
There was a problem hiding this comment.
why remove addPlanProcesses?
Contributor
Author
There was a problem hiding this comment.
假设plan1 被costBaseRule 改写为 plan2， 然后再被后续rule（不包含其他costBasedRule）改写为plan3
如果选中了costBaseRule的改写，那么退出costBaseRule后，我们应该从plan2继续改写。这里的plan process 记录的是 plan2 到plan3 的过程
| 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); |
Contributor
There was a problem hiding this comment.
only process RewrittenCteProducer is enough? maybe we should process all cte related context just like what we do in ClearContextStatus
Contributor
Author
There was a problem hiding this comment.
增加了CteEnvironmentSnapshot, 恢复所有cte相关数据
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 31683 ms |
TPC-DS: Total hot run time: 172076 ms |
ClickBench: Total hot run time: 26.51 s |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
cost based rewrite job may contaminate StatementContext.rewrittenCteProducer
fix solution: clone current rewrittenCteProducer, and restore it after each call to getCost(...)
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)