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.

Sign up for GitHub

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

Jump to bottom

privilege: add drop_priv check when truncate table #9870

Merged
merged 6 commits into from
Mar 24, 2019
Merged

privilege: add drop_priv check when truncate table #9870

merged 6 commits into from
Mar 24, 2019

Conversation

winkyao
Copy link
Contributor

@winkyao winkyao commented Mar 24, 2019 •
edited
Loading

What problem does this PR solve?

Before this PR, TiDB user truncate table only needs delete_priv. After this PR, needs drop_priv. To be consistent with MySQL. https://dev.mysql.com/doc/refman/5.7/en/privileges-provided.html#priv_drop

What is changed and how it works?

Add drop_priv to the visitInfo in the plan builder.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

@winkyao
Copy link
Contributor Author

winkyao commented Mar 24, 2019

@tiancaiamao PTAL

@winkyao
Copy link
Contributor Author

winkyao commented Mar 24, 2019

@xiekeyi98 PTAL

@codecov
Copy link

codecov bot commented Mar 24, 2019 •
edited
Loading

Codecov Report

Merging #9870 into master will increase coverage by 0.0037%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master      #9870        +/-   ##
================================================
+ Coverage   67.3504%   67.3541%   +0.0037%     
================================================
  Files           383        383                
  Lines         80436      80436                
================================================
+ Hits          54174      54177         +3     
- Misses        21416      21417         +1     
+ Partials       4846       4842         -4

xiekeyi98
xiekeyi98 reviewed Mar 24, 2019
View reviewed changes
Copy link
Contributor

@xiekeyi98 xiekeyi98 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Do we only need drop privilege
Because :
image

Maybe we could delete delete privilege?

Rest LGTM

@xiekeyi98 xiekeyi98 mentioned this pull request Mar 24, 2019
Merged
@winkyao
Copy link
Contributor Author

winkyao commented Mar 24, 2019

@xiekeyi98 Done.

xiekeyi98
xiekeyi98 reviewed Mar 24, 2019
View reviewed changes
Copy link
Contributor

@xiekeyi98 xiekeyi98 left a comment

Choose a reason for hiding this comment

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

LGTM

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

LGTM

@tiancaiamao
Copy link
Contributor

/run-all-tests

@tiancaiamao tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 24, 2019
@winkyao
Copy link
Contributor Author

winkyao commented Mar 24, 2019

/run-all-tests

crazycs520
crazycs520 approved these changes Mar 24, 2019
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520 crazycs520 merged commit 630671e into master Mar 24, 2019
@tiancaiamao tiancaiamao deleted the truncate_priv branch March 25, 2019 01:37
@winkyao winkyao mentioned this pull request Mar 25, 2019
Merged
zz-jason pushed a commit that referenced this pull request Mar 25, 2019
@winkyao @zz-jason
privilege: add drop_priv check when truncate table (#9870) (#9876)
f9c995a
@xiekeyi98 xiekeyi98 mentioned this pull request Mar 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiekeyi98 xiekeyi98 xiekeyi98 left review comments

@crazycs520 crazycs520 crazycs520 approved these changes

@tiancaiamao tiancaiamao Awaiting requested review from tiancaiamao

Assignees
No one assigned
Labels
status/LGT2 Indicates that a PR has LGTM 2. type/compatibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@winkyao @tiancaiamao @xiekeyi98 @crazycs520