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

executor, plan: add executor to support GRANT ROLE #9721

Merged
merged 12 commits into from
Mar 30, 2019

Conversation

imtbkcat
Copy link

What problem does this PR solve?

add executor to support SQL like GRANT roles TO users.

What is changed and how it works?

Adding executeGrantRole for SimpleExec executor. Checking whether all roles and users exist before grant. When executing GRANT ROLE, executor will insert tuple into mysql.role_edges table and flush privilege.

Test case will be added later.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • Increased code complexity

@imtbkcat
Copy link
Author

parser pr: pingcap/parser#242

@codecov
Copy link

codecov bot commented Mar 14, 2019

Codecov Report

Merging #9721 into master will decrease coverage by 0.0067%.
The diff coverage is 54.2857%.

@@               Coverage Diff                @@
##             master      #9721        +/-   ##
================================================
- Coverage   77.5494%   77.5426%   -0.0068%     
================================================
  Files           404        404                
  Lines         81704      81706         +2     
================================================
- Hits          63361      63357         -4     
+ Misses        13649      13646         -3     
- Partials       4694       4703         +9

@imtbkcat imtbkcat added sig/execution SIG execution type/TEP sig/planner SIG: Planner labels Mar 15, 2019
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 15, 2019
@tiancaiamao
Copy link
Contributor

PTAL @jackysp @lysu

@jackysp
Copy link
Member

jackysp commented Mar 22, 2019

Please resolve the conflicts.

@imtbkcat
Copy link
Author

/rebuild

@imtbkcat
Copy link
Author

/run-all-tests

executor/simple.go Outdated Show resolved Hide resolved
executor/simple.go Outdated Show resolved Hide resolved
executor/simple.go Show resolved Hide resolved
executor/simple.go Outdated Show resolved Hide resolved
executor/simple.go Outdated Show resolved Hide resolved
executor/simple.go Outdated Show resolved Hide resolved
executor/simple.go Outdated Show resolved Hide resolved
@imtbkcat
Copy link
Author

PTAL @zz-jason

@imtbkcat
Copy link
Author

/run-all-tests

@imtbkcat
Copy link
Author

/run-all-tests

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 30, 2019
@zz-jason zz-jason merged commit c612252 into pingcap:master Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants