-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
plan: refine code about maxonerow #5501
Conversation
plan/plan.go
Outdated
case *LogicalMaxOneRow, *LogicalExists: | ||
p.maxOneRow = true | ||
} | ||
if len(p.children) == 1 { | ||
switch p.self.(type) { | ||
case *LogicalExists, *LogicalAggregation, *LogicalProjection: | ||
p.basePlan.schema.Keys = nil | ||
case *LogicalLock: | ||
p.basePlan.schema.Keys = p.basePlan.children[0].Schema().Keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need basePlan too?
{ | ||
// Test MaxOneRow for limit. | ||
sql: "select (select * from (select b from t limit 1) x where x.b = t1.b) from t t1", | ||
best: "Join{DataScan(t1)->DataScan(t)->Projection->Limit}(t1.b,x.b)->Projection->Projection", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we tell there is a MaxOneRow attribute in the plan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this test, the plan contains an Apply op.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
LGTM |
/run-common-test |
/run-all-tests |
@hanfei1991 Please fix CI. |
/run-common-test -tidb-test=pr/439 |
/run-integration-common-test -tidb-test=pr/439 |
move maxOneRow out of schema to do further optimize