-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: support privilege management #54
Conversation
@@ -7648,6 +7766,7 @@ keyword_as_identifier: | |||
| "RETURNS" | |||
| "RETURN" | |||
| "REVOKE" | |||
| "ROLE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pg merge user
and group
into single role
, user just role with LOGIN attribute. This have many advantages, ref https://stackoverflow.com/a/8499491
allow groups to have groups as members
the ACL code would be simplified
the GRANT/REVOKE syntax and the display format for ACL lists could be simplified, since there'd be no need for a syntactic marker as to whether a given name is a user or a group.
In some circumstances I could see it making sense to allow logging in directly as a group/role/whatchacallit
This would also solve the problem that information_schema views will show only owned objects
[makes it easier to] representing privileges granted to groups [since you'd simply reuse the role related code?]
At least simplification make sense, e.g the privilege list. Consider ?
{ | ||
$$ = parser->MakeIdentifier(@1, parser->GetInputText(@1)); | ||
} | ||
| "ALTER" "USER" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just concern about extensibility.
Those two word privileges do require parser update for every new privilege, or it better to replace e.g 'CREATE USER' to 'CREATEUSER'.
Anyway you can reserve some common parts of the two words privileges, as synonyms. MySQL dialect is ugly :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combine the two words looks more strange. how to set as synonyms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not parser side, but privilege impl. Just point it out here
support the following syntax: