-
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
planner: fix update privilege error #10085
planner: fix update privilege error #10085
Conversation
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.
Thanks for your contribution.
planner/core/logical_plan_builder.go
Outdated
@@ -2611,6 +2614,13 @@ func (b *PlanBuilder) buildUpdateLists(tableList []*ast.TableName, list []*ast.A | |||
col := assign.Col | |||
|
|||
dbName := col.DBName.L | |||
// With sql below we need to get database by table alia name |
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.
What the alia
stand for?
// With sql below we need to get database by table alia name | |
// With sql below, we need to get database by the table alias name. |
planner/core/logical_plan_builder.go
Outdated
@@ -2611,6 +2614,13 @@ func (b *PlanBuilder) buildUpdateLists(tableList []*ast.TableName, list []*ast.A | |||
col := assign.Col | |||
|
|||
dbName := col.DBName.L | |||
// With sql below we need to get database by table alia name | |||
// update ap.record t inner join tp.record tt on t.id=tt.id set t.name=tt.name |
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.
// update ap.record t inner join tp.record tt on t.id=tt.id set t.name=tt.name | |
// issue#10028 |
Codecov Report
@@ Coverage Diff @@
## master #10085 +/- ##
================================================
- Coverage 77.3341% 77.3312% -0.0029%
================================================
Files 412 412
Lines 85622 85629 +7
================================================
+ Hits 66215 66218 +3
- Misses 14389 14390 +1
- Partials 5018 5021 +3 |
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 |
planner/core/logical_plan_builder.go
Outdated
@@ -2611,6 +2614,12 @@ func (b *PlanBuilder) buildUpdateLists(tableList []*ast.TableName, list []*ast.A | |||
col := assign.Col | |||
|
|||
dbName := col.DBName.L | |||
// To solve issue#10028, we need to get database name by the table alias name. | |||
for _, tbl := range tableList { |
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.
will it be faster if we pre-build a map[tblName]DBName
and save this for
loop?
planner/core/logical_plan_builder.go
Outdated
// To solve issue#10028, we need to get database name by the table alias name. | ||
for _, tbl := range tableList { | ||
if strings.EqualFold(tbl.Name.L, col.TblName.L) { | ||
dbName = tbl.DBInfo.Name.L |
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.
BTW, after finding a dbName, we should break
.
@zz-jason |
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 |
/run-mybatis-test |
@haplone Please cherry pick this commit to the release-2.1 branch |
@zz-jason func (b *planBuilder) buildUpdate(update *ast.UpdateStmt) (Plan, error) {
// ...
for _, t := range tableList {
dbName := t.Schema.L
if dbName == "" {
dbName = b.ctx.GetSessionVars().CurrentDB
}
// the mysql.UpdatePriv should be mysql.SelectPriv
b.visitInfo = appendVisitInfo(b.visitInfo, mysql.UpdatePriv, dbName, t.Name.L, "")
} |
seems we need to cherry pick #8376 to release 2.1 first @tiancaiamao |
What problem does this PR solve?
issue10028
What is changed and how it works?
get the right database name for update privilege check in planner/core/logical_plan_builder.go#buildUpdate
Check List
Tests
Code changes
Side effects
Related changes
we need to fix in release-2.0 release-2.1 by another pr(buildUpdate logic has changed)