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

feat: support privilege management #54

Closed
wants to merge 10 commits into from
Closed

feat: support privilege management #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
65db553
feat: add privilege name
dl239 Feb 21, 2024
064bf06
feat: support grant option
dl239 Feb 22, 2024
b8754dc
test: add show grants test
dl239 Feb 22, 2024
0dfd044
feat: add object type
dl239 Feb 27, 2024
9cb8c48
feat: support grant path
dl239 Mar 4, 2024
05d08e2
fix: fix comment
dl239 Mar 6, 2024
e925fe0
update enum
dl239 Mar 7, 2024
c0915f8
fix: revert grant_path in grant statement class
dl239 Mar 8, 2024
eabf0e6
fix: fix revoke
dl239 Mar 8, 2024
baec6c3
refact: rm grant path node
dl239 Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 124 additions & 8 deletions zetasql/parser/bison_parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,7 @@ using zetasql::ASTDropStatement;
%token KW_CURRENT_ROW "CURRENT_ROW"
%token KW_DATA "DATA"
%token KW_DATABASE "DATABASE"
%token KW_DATABASES "DATABASES"
%token KW_DATE "DATE"
%token KW_DATETIME "DATETIME"
%token KW_DECIMAL "DECIMAL"
Expand Down Expand Up @@ -880,6 +881,7 @@ using zetasql::ASTDropStatement;
%token KW_NUMERIC "NUMERIC"
%token KW_OFFSET "OFFSET"
%token KW_ONLY "ONLY"
%token KW_OPTION "OPTION"
%token KW_OPTIONS "OPTIONS"
%token KW_OUT "OUT"
%token KW_OUTFILE "OUTFILE"
Expand Down Expand Up @@ -908,6 +910,7 @@ using zetasql::ASTDropStatement;
%token KW_RETURN "RETURN"
%token KW_RETURNS "RETURNS"
%token KW_REVOKE "REVOKE"
%token KW_ROLE "ROLE"
%token KW_ROLLBACK "ROLLBACK"
%token KW_ROW "ROW"
%token KW_RUN "RUN"
Expand Down Expand Up @@ -1034,6 +1037,7 @@ using zetasql::ASTDropStatement;
%type <expression> expression
%type <node> generic_entity_type
%type <node> grant_to_clause
%type <expression> grant_path
%type <node> index_storing_expression_list_prefix
%type <node> index_storing_expression_list
%type <expression> interval_expression
Expand Down Expand Up @@ -1078,6 +1082,7 @@ using zetasql::ASTDropStatement;
%type <node> hint_with_body_prefix
%type <identifier> identifier
%type <identifier> identifier_in_hints
%type <identifier> star_or_identifier
%type <node> if_statement
%type <node> elseif_clauses
%type <node> execute_immediate
Expand Down Expand Up @@ -1394,6 +1399,7 @@ using zetasql::ASTDropStatement;
%type <all_or_distinct_keyword> all_or_distinct
%type <all_or_distinct_keyword> opt_all_or_distinct
%type <schema_object_kind_keyword> schema_object_kind
%type <schema_object_kind_keyword> grant_object_kind

%type <not_keyword_presence> between_operator
%type <not_keyword_presence> in_operator
Expand All @@ -1418,6 +1424,7 @@ using zetasql::ASTDropStatement;
%type <boolean> opt_filter
%type <boolean> opt_if_exists
%type <boolean> opt_if_not_exists
%type <boolean> opt_grant_option
%type <boolean> opt_natural
%type <boolean> opt_not_aggregate
%type <boolean> opt_or_replace
Expand Down Expand Up @@ -3375,27 +3382,45 @@ export_model_statement:
;

grant_statement:
"GRANT" privileges "ON" identifier path_expression "TO" grantee_list
"GRANT" privileges "ON" grant_object_kind grant_path "TO" grantee_list opt_grant_option
aceforeverd marked this conversation as resolved.
Show resolved Hide resolved
dl239 marked this conversation as resolved.
Show resolved Hide resolved
aceforeverd marked this conversation as resolved.
Show resolved Hide resolved
{
$$ = MAKE_NODE(ASTGrantStatement, @$, {$2, $4, $5, $7});
auto grant_statement = MAKE_NODE(ASTGrantStatement, @$, {$2, $5, $7});
grant_statement->set_object_kind($4);
grant_statement->set_is_with_grant_option($8);
$$ = grant_statement;
}
| "GRANT" privileges "ON" path_expression "TO" grantee_list
| "GRANT" privileges "ON" grant_path "TO" grantee_list opt_grant_option
{
$$ = MAKE_NODE(ASTGrantStatement, @$, {$2, $4, $6});
auto grant_statement = MAKE_NODE(ASTGrantStatement, @$, {$2, $4, $6});
grant_statement->set_is_with_grant_option($7);
$$ = grant_statement;
}
;

revoke_statement:
"REVOKE" privileges "ON" identifier path_expression "FROM" grantee_list
dl239 marked this conversation as resolved.
Show resolved Hide resolved
"REVOKE" privileges "ON" grant_object_kind grant_path "FROM" grantee_list
{
$$ = MAKE_NODE(ASTRevokeStatement, @$, {$2, $4, $5, $7});
auto revoke_statement = MAKE_NODE(ASTRevokeStatement, @$, {$2, $5, $7});
revoke_statement->set_object_kind($4);
$$ = revoke_statement;
}
| "REVOKE" privileges "ON" path_expression "FROM" grantee_list
| "REVOKE" privileges "ON" grant_path "FROM" grantee_list
{
$$ = MAKE_NODE(ASTRevokeStatement, @$, {$2, $4, $6});
}
;

grant_object_kind:
"TABLE"
{ $$ = zetasql::SchemaObjectKind::kTable; }
| "FUNCTION"
{ $$ = zetasql::SchemaObjectKind::kFunction; }
| "DEPLOYMENT"
{ $$ = zetasql::SchemaObjectKind::kDeployment; }
| "VIEW"
{ $$ = zetasql::SchemaObjectKind::kView; }
;

privileges:
"ALL" opt_privileges_keyword
{
Expand Down Expand Up @@ -3435,11 +3460,99 @@ privilege_name:
{
$$ = $1;
}
| KW_SELECT
| "SELECT"
{
// The SELECT keyword is allowed to be a privilege name.
$$ = parser->MakeIdentifier(@1, parser->GetInputText(@1));
}
| "CREATE"
{
$$ = parser->MakeIdentifier(@1, parser->GetInputText(@1));
}
| "INDEX"
{
$$ = parser->MakeIdentifier(@1, parser->GetInputText(@1));
}
| "ALTER" "USER"
Copy link
Collaborator

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 :(

Copy link
Collaborator Author

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

Copy link
Collaborator

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

{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "CREATE" "USER"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "CREATE" "ROLE"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "DROP" "DEPLOYMENT"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "DROP" "USER"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "DROP" "ROLE"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "SHOW" "DATABASES"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
| "GRANT" "OPTION"
{
std::string identifier = absl::StrCat(parser->GetInputText(@1), " ", parser->GetInputText(@2));
$$ = parser->MakeIdentifier(@$, identifier.c_str());
}
;

star_or_identifier:
"*"
{
$$ = parser->MakeIdentifier(@1, parser->GetInputText(@1));
}
| identifier
{
$$ = $1;
}
;

grant_path:
star_or_identifier
{
$$ = MAKE_NODE(ASTPathExpression, @$, {$1});
}
| star_or_identifier ".*"
{
auto path_expression = MAKE_NODE(ASTPathExpression, @$, {$1});
auto id = parser->MakeIdentifier(@2, "*");
path_expression->AddChild(id);
$$ = path_expression;
}
| grant_path "." star_or_identifier
aceforeverd marked this conversation as resolved.
Show resolved Hide resolved
{
$$ = WithEndLocation(WithExtraChildren($1, {$3}), @$);
}
;

opt_grant_option:
"WITH" "GRANT" "OPTION"
{
$$ = true;
}
| /* Nothing */
{
$$ = false;
}
;

rename_statement:
Expand Down Expand Up @@ -7557,6 +7670,7 @@ keyword_as_identifier:
| "CURRENT_ROW"
| "DATA"
| "DATABASE"
| "DATABASES"
| "DATE"
| "DATETIME"
| "DECIMAL"
Expand Down Expand Up @@ -7620,6 +7734,7 @@ keyword_as_identifier:
| "NUMERIC"
| "OFFSET"
| "ONLY"
| "OPTION"
| "OPTIONS"
| "OUT"
| "OUTFILE"
Expand Down Expand Up @@ -7648,6 +7763,7 @@ keyword_as_identifier:
| "RETURNS"
| "RETURN"
| "REVOKE"
| "ROLE"
Copy link
Collaborator

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 ?

| "ROLLBACK"
| "ROW"
| "RUN"
Expand Down
3 changes: 3 additions & 0 deletions zetasql/parser/flex_tokenizer.l
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ current_time { return BisonParserImpl::token::KW_CURRENT_TIME;}
current_row { return BisonParserImpl::token::KW_CURRENT_ROW; }
data { return BisonParserImpl::token::KW_DATA; }
database { return BisonParserImpl::token::KW_DATABASE; }
databases { return BisonParserImpl::token::KW_DATABASES; }
date { return BisonParserImpl::token::KW_DATE; }
datetime { return BisonParserImpl::token::KW_DATETIME; }
decimal { return BisonParserImpl::token::KW_DECIMAL; }
Expand Down Expand Up @@ -566,6 +567,7 @@ offset { return BisonParserImpl::token::KW_OFFSET; }
on { return BisonParserImpl::token::KW_ON; }
only { return BisonParserImpl::token::KW_ONLY; }
open { return BisonParserImpl::token::KW_OPEN;}
option { return BisonParserImpl::token::KW_OPTION; }
options { return BisonParserImpl::token::KW_OPTIONS; }
or { return BisonParserImpl::token::KW_OR; }
order { return BisonParserImpl::token::KW_ORDER; }
Expand Down Expand Up @@ -603,6 +605,7 @@ return { return BisonParserImpl::token::KW_RETURN; }
returns { return BisonParserImpl::token::KW_RETURNS; }
revoke { return BisonParserImpl::token::KW_REVOKE; }
right { return BisonParserImpl::token::KW_RIGHT; }
role { return BisonParserImpl::token::KW_ROLE; }
rollback { return BisonParserImpl::token::KW_ROLLBACK; }
rollup { return BisonParserImpl::token::KW_ROLLUP; }
row { return BisonParserImpl::token::KW_ROW; }
Expand Down
3 changes: 3 additions & 0 deletions zetasql/parser/keywords.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ constexpr KeywordInfoPOD kAllKeywords[] = {
{"current_row", KW_CURRENT_ROW},
{"data", KW_DATA},
{"database", KW_DATABASE},
{"databases", KW_DATABASES},
{"date", KW_DATE},
{"datetime", KW_DATETIME},
{"decimal", KW_DECIMAL},
Expand Down Expand Up @@ -221,6 +222,7 @@ constexpr KeywordInfoPOD kAllKeywords[] = {
{"on", KW_ON, KeywordInfo::kReserved},
{"only", KW_ONLY},
{"open", KW_OPEN, KeywordInfo::kReserved},
{"option", KW_OPTION},
{"options", KW_OPTIONS},
{"or", KW_OR, KeywordInfo::kReserved},
{"order", KW_ORDER, KeywordInfo::kReserved},
Expand Down Expand Up @@ -259,6 +261,7 @@ constexpr KeywordInfoPOD kAllKeywords[] = {
{"returns", KW_RETURNS},
{"revoke", KW_REVOKE},
{"right", KW_RIGHT, KeywordInfo::kReserved},
{"role", KW_ROLE},
{"rollback", KW_ROLLBACK},
{"rollup", KW_ROLLUP, KeywordInfo::kReserved},
{"row", KW_ROW},
Expand Down
25 changes: 19 additions & 6 deletions zetasql/parser/parse_tree_manual.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6839,23 +6839,31 @@ class ASTGrantStatement final : public ASTStatement {
NonRecursiveParseTreeVisitor* visitor) const override;

const ASTPrivileges* privileges() const { return privileges_; }
const ASTIdentifier* target_type() const { return target_type_; }
const ASTPathExpression* target_path() const { return target_path_; }
const ASTGranteeList* grantee_list() const { return grantee_list_; }
bool is_with_grant_option() const { return is_with_grant_option_; }
void set_is_with_grant_option(bool value) { is_with_grant_option_ = value; }
const SchemaObjectKind object_kind() const {
return object_kind_;
}
void set_object_kind(SchemaObjectKind object_kind) {
object_kind_ = object_kind;
}

private:
void InitFields() final {
FieldLoader fl(this);
fl.AddRequired(&privileges_);
fl.AddOptional(&target_type_, AST_IDENTIFIER);
fl.AddRequired(&target_path_);
fl.AddRequired(&grantee_list_);
}

const ASTPrivileges* privileges_ = nullptr; // Required
const ASTIdentifier* target_type_ = nullptr; // Optional
SchemaObjectKind object_kind_ =
SchemaObjectKind::kInvalidSchemaObjectKind;
const ASTPathExpression* target_path_ = nullptr; // Required
const ASTGranteeList* grantee_list_ = nullptr; // Required
bool is_with_grant_option_ = false;
};

class ASTRevokeStatement final : public ASTStatement {
Expand All @@ -6868,21 +6876,26 @@ class ASTRevokeStatement final : public ASTStatement {
NonRecursiveParseTreeVisitor* visitor) const override;

const ASTPrivileges* privileges() const { return privileges_; }
const ASTIdentifier* target_type() const { return target_type_; }
const ASTPathExpression* target_path() const { return target_path_; }
const ASTGranteeList* grantee_list() const { return grantee_list_; }
const SchemaObjectKind object_kind() const {
return object_kind_;
}
void set_object_kind(SchemaObjectKind object_kind) {
object_kind_ = object_kind;
}

private:
void InitFields() final {
FieldLoader fl(this);
fl.AddRequired(&privileges_);
fl.AddOptional(&target_type_, AST_IDENTIFIER);
fl.AddRequired(&target_path_);
fl.AddRequired(&grantee_list_);
}

const ASTPrivileges* privileges_ = nullptr;
const ASTIdentifier* target_type_ = nullptr;
SchemaObjectKind object_kind_ =
SchemaObjectKind::kInvalidSchemaObjectKind;
const ASTPathExpression* target_path_ = nullptr;
const ASTGranteeList* grantee_list_ = nullptr;
};
Expand Down
Loading
Loading