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: using system session to execute internal sql of RBAC #13820

Merged
merged 3 commits into from
Dec 4, 2019

Conversation

imtbkcat
Copy link

What problem does this PR solve?

Some RBAC's internal sql were executed by SQLExecutor.Execute, which will be recored by binlog. Interal sql should not recored by binlog.

What is changed and how it works?

Use system session to execute all interal sql of RBAC.

Check List

Tests

  • Unit test

Code changes

  • None

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

@codecov
Copy link

codecov bot commented Nov 29, 2019

Codecov Report

Merging #13820 into master will decrease coverage by 0.0907%.
The diff coverage is 42.4242%.

@@               Coverage Diff                @@
##             master     #13820        +/-   ##
================================================
- Coverage   80.2264%   80.1356%   -0.0908%     
================================================
  Files           474        474                
  Lines        117647     117200       -447     
================================================
- Hits          94384      93919       -465     
- Misses        15839      15851        +12     
- Partials       7424       7430         +6

@imtbkcat
Copy link
Author

/run-all-tests

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 2, 2019
@imtbkcat
Copy link
Author

imtbkcat commented Dec 2, 2019

/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

@jackysp jackysp added the status/can-merge Indicates a PR has been approved by a committer. label Dec 4, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

Your auto merge job has been accepted, waiting for 13852

@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

/run-all-tests

@sre-bot sre-bot merged commit 527c1ae into pingcap:master Dec 4, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

cherry pick to release-3.0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/privilege component/session status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants