Skip to content
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

privilege: fix update privilege error for release-2.1 #10104

Closed

Conversation

haplone
Copy link
Contributor

@haplone haplone commented Apr 10, 2019

What problem does this PR solve?

issue10028

update ap.record t inner join tp.record tt on t.id=tt.id  set t.name=tt.name;
ERROR 1105 (HY000): privilege check fail

What is changed and how it works?

only use modified table name for update privilege check in planner/core/logical_plan_builder.go#buildUpdate

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

@haplone
Copy link
Contributor Author

haplone commented Apr 22, 2019

@imtbkcat
I found branch release 2.1 on tidb has an error, if it is fixed ,we could solve this issue in release 2.1 by cherry pick :
https://github.com/pingcap/tidb/blob/release-2.1/planner/core/logical_plan_builder.go#L2153

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, "")
	}

@zz-jason
Copy link
Member

@haplone Is this problem existed in the master branch?

@zz-jason zz-jason added component/privilege contribution This PR is from a community contributor. labels Apr 22, 2019
@haplone
Copy link
Contributor Author

haplone commented Apr 23, 2019

@haplone Is this problem existed in the master branch?

Yes, this problem existed in the master branch.

we could see issue10028 and PR10085

@zz-jason

@haplone
Copy link
Contributor Author

haplone commented May 5, 2019

@zz-jason

this problem exists in the master branch

PTAL

@zz-jason
Copy link
Member

zz-jason commented May 5, 2019

@zz-jason

this problem exists in the master branch

PTAL

OK, How about getting the master PR merged first then do the cherry-pick

@qw4990
Copy link
Contributor

qw4990 commented May 13, 2019

@haplone PTAL, could you please fix these conflicts and help us merge this PR into release 2.1?

@haplone haplone closed this May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/privilege contribution This PR is from a community contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants