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

ast: add role support show grant #272

Merged
merged 3 commits into from
Apr 12, 2019
Merged

ast: add role support show grant #272

merged 3 commits into from
Apr 12, 2019

Conversation

imtbkcat
Copy link

@imtbkcat imtbkcat commented Apr 3, 2019

What problem does this PR solve?

Adding SHOW GRANT USING roles support.

What is changed and how it works?

Adding Roles field for ShowStmt, which store information in UsingOption.

Check List

Tests

  • Unit test

Code changes

  • Has exported variable/fields change

Side effects

  • Increased code complexity

@imtbkcat imtbkcat mentioned this pull request Apr 4, 2019
Merged
jackysp
jackysp reviewed Apr 4, 2019
View reviewed changes
parser.y Outdated
@@ -854,6 +854,7 @@ import (
UserSpec "Username and auth option"
UserSpecList "Username and auth option list"
UserVariableList "User defined variable name list"
UsingRoles "UsingRoles is role option for SHOW GRANT"
Copy link
Member

Choose a reason for hiding this comment

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

Please keep aligning.

Copy link
Author

Choose a reason for hiding this comment

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

@jackysp fixed.

jackysp
jackysp reviewed Apr 4, 2019
View reviewed changes
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.

rest LGTM

@imtbkcat imtbkcat force-pushed the showgrant branch 3 times, most recently from 76648a9 to 74eb848 Compare April 9, 2019 03:34
@CLAassistant
Copy link

CLAassistant commented Apr 12, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

kennytm
kennytm reviewed Apr 12, 2019
View reviewed changes
ast/dml.go Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
kennytm
kennytm approved these changes Apr 12, 2019
View reviewed changes
Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@imtbkcat
Copy link
Author

@kennytm Please merge this pr, because i dont have write access privilege.

@kennytm kennytm added the status/LGT2 LGT2 label Apr 12, 2019
@kennytm kennytm merged commit 500a7c8 into pingcap:master Apr 12, 2019
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
@kennytm
ast: add role support show grant (pingcap#272)
00eef86 
* add option for show grant

* fix tab

* address comment
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
@kennytm
ast: add role support show grant (pingcap#272)
6a2dce8 
* add option for show grant

* fix tab

* address comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackysp jackysp jackysp left review comments

@kennytm kennytm kennytm approved these changes

Assignees
No one assigned
Labels
status/LGT2 LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@imtbkcat @CLAassistant @kennytm @jackysp