-
Notifications
You must be signed in to change notification settings - Fork 6.2k
planner: lateral join quality updates #67482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
36790f8
6f70fa8
a05b034
cfd175a
abdbca3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,8 +218,8 @@ func (p *LogicalCTE) DeriveStats(_ []*property.StatsInfo, selfSchema *expression | |
| vars := p.SCtx().GetSessionVars() | ||
| savedParallelApply := vars.EnableParallelApply | ||
| vars.EnableParallelApply = false | ||
| defer func() { vars.EnableParallelApply = savedParallelApply }() | ||
| _, p.Cte.RecursivePartPhysicalPlan, _, err = utilfuncp.DoOptimize(context.TODO(), p.SCtx(), p.Cte.OptFlag, p.Cte.RecursivePartLogicalPlan) | ||
| vars.EnableParallelApply = savedParallelApply | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, will the original code cause any issues?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From a deeper analysis - the original code works correctly in a normal execution. However, it is not panic-safe. If DoOptimize panics, then the original code would leak EnableParallelApply = false for the rest of the session. It is not clear to me how many users enable parallel Apply currently. And this fix is already in the cherry pick branch (where this bug was found). And it is unlikely that anyone else has this issue - since lateral join fix was recently merged to master. |
||
| if err != nil { | ||
| return nil, false, err | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.