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

parser: add DROP ROLE support #237

Merged
merged 3 commits into from
Mar 12, 2019
Merged

parser: add DROP ROLE support #237

merged 3 commits into from
Mar 12, 2019

Conversation

imtbkcat
Copy link

@imtbkcat imtbkcat commented Mar 8, 2019

What problem does this PR solve?

add DROP ROLE function for parser and executor.

What is changed and how it works?

Role is same as user, is store in mysql.user table. We do same action as DROP USER when we need to drop a role. Also, as we has added some system table for RBAC, like mysql.role_edges and
mysql.default_role. We have to delete information from these tables when execute DROP ROLE

tidb pr: pingcap/tidb#9616

Check List

Tests

  • Unit test

Code changes

  • Has exported variable/fields change

Side effects

  • Increased code complexity

@imtbkcat imtbkcat changed the title executor: add DROP ROLE support parser: add DROP ROLE support Mar 8, 2019
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

@@ -2429,19 +2429,31 @@ DropViewStmt:
DropUserStmt:
"DROP" "USER" UsernameList
{
$$ = &ast.DropUserStmt{IfExists: false, UserList: $3.([]*auth.UserIdentity)}
$$ = &ast.DropUserStmt{IsDropRole: false, IfExists: false, UserList: $3.([]*auth.UserIdentity)}
Copy link
Member

Choose a reason for hiding this comment

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

how about using a new DropRoleStmt?

Copy link
Author

Choose a reason for hiding this comment

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

actually there is no difference between DROP USER and DROP ROLE. I think IsDropRole can be removed.

@kennytm kennytm added the status/LGT1 LGT1 label Mar 9, 2019
@tiancaiamao
Copy link
Collaborator

LGTM

@tiancaiamao tiancaiamao added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Mar 12, 2019
@jackysp jackysp merged commit 3f6280b into pingcap:master Mar 12, 2019
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants