From e724e972fe8455aa2197264b14d45e1ddd22d3ff Mon Sep 17 00:00:00 2001 From: Caspian Date: Fri, 23 Jul 2021 21:37:55 +0800 Subject: [PATCH] remove uncessary Exception --- src/Parsers/ASTRolesOrUsersSet.h | 2 -- src/Parsers/ParserGrantQuery.cpp | 35 +++---------------- src/Parsers/ParserRolesOrUsersSet.cpp | 19 +++------- .../01999_grant_with_replace.reference | 8 +++++ .../0_stateless/01999_grant_with_replace.sql | 17 ++++++++- 5 files changed, 33 insertions(+), 48 deletions(-) diff --git a/src/Parsers/ASTRolesOrUsersSet.h b/src/Parsers/ASTRolesOrUsersSet.h index ab2d9bb4a00..15d42ee39a0 100644 --- a/src/Parsers/ASTRolesOrUsersSet.h +++ b/src/Parsers/ASTRolesOrUsersSet.h @@ -25,8 +25,6 @@ public: bool id_mode = false; /// whether this set keep UUIDs instead of names bool use_keyword_any = false; /// whether the keyword ANY should be used instead of the keyword ALL - bool none_role_parsed = false; /// whether keyword NONE has been parsed - bool empty() const { return names.empty() && !current_user && !all; } void replaceCurrentUserTag(const String & current_user_name); diff --git a/src/Parsers/ParserGrantQuery.cpp b/src/Parsers/ParserGrantQuery.cpp index 5ea6ac0ec92..bd4b07bb7b3 100644 --- a/src/Parsers/ParserGrantQuery.cpp +++ b/src/Parsers/ParserGrantQuery.cpp @@ -293,41 +293,14 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) bool replace_access = false; + bool replace_role = false; if (is_replace) { if (roles) - { // assigning role mode - if (!roles->empty() && roles->none_role_parsed) - throw Exception("In assigning role WITH REPLACE OPTION sql, 'NONE' can only be used alone to rovoke all roles", ErrorCodes::SYNTAX_ERROR); - } + replace_role = true; else - { - // granting privilege mode replace_access = true; - bool new_access = false; - bool none_on_all = false; - for (auto & element : elements) - { - if (element.access_flags.isEmpty()) - { - if (element.any_database) - none_on_all = true; - else - throw Exception("In granting privilege WITH REPLACE OPTION sql, 'NONE ON db.*' should be 'NONE ON *.*', and can only be used alone to drop all privileges on any database", ErrorCodes::SYNTAX_ERROR); - } - else - { - new_access = true; - } - } - - if (new_access && none_on_all) - throw Exception("In granting privilege WITH REPLACE OPTION sql, 'NONE ON db.*' should be 'NONE ON *.*', and can only be used alone to drop all privileges on any database", ErrorCodes::SYNTAX_ERROR); - } - } - - if (is_replace && !replace_access && roles && !roles->empty() && roles->none_role_parsed) - throw Exception("In REPLACE GRANT assigning role sql, 'NONE' can only be used alone to rovoke all roles", ErrorCodes::SYNTAX_ERROR); + } if (!is_revoke) eraseNonGrantable(elements); @@ -343,7 +316,7 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) query->grantees = std::move(grantees); query->admin_option = admin_option; query->replace_access = replace_access; - query->replace_granted_roles = (is_replace && !replace_access); + query->replace_granted_roles = replace_role; return true; } diff --git a/src/Parsers/ParserRolesOrUsersSet.cpp b/src/Parsers/ParserRolesOrUsersSet.cpp index c5a459fd199..41e9ee6501d 100644 --- a/src/Parsers/ParserRolesOrUsersSet.cpp +++ b/src/Parsers/ParserRolesOrUsersSet.cpp @@ -44,22 +44,17 @@ namespace bool allow_current_user, bool & all, Strings & names, - bool & current_user, - bool & none_role_parsed) + bool & current_user) { bool res_all = false; Strings res_names; bool res_current_user = false; Strings res_with_roles_names; - bool parsed_none = false; auto parse_element = [&] { if (ParserKeyword{"NONE"}.ignore(pos, expected)) - { - parsed_none = true; return true; - } if (allow_all && ParserKeyword{"ALL"}.ignore(pos, expected)) { @@ -95,7 +90,6 @@ namespace names = std::move(res_names); current_user = res_current_user; all = res_all; - none_role_parsed = parsed_none; return true; } @@ -105,15 +99,14 @@ namespace bool id_mode, bool allow_current_user, Strings & except_names, - bool & except_current_user, - bool & parsed_none) + bool & except_current_user) { return IParserBase::wrapParseImpl(pos, [&] { if (!ParserKeyword{"EXCEPT"}.ignore(pos, expected)) return false; bool unused; - return parseBeforeExcept(pos, expected, id_mode, false, false, allow_current_user, unused, except_names, except_current_user, parsed_none); + return parseBeforeExcept(pos, expected, id_mode, false, false, allow_current_user, unused, except_names, except_current_user); }); } } @@ -126,12 +119,11 @@ bool ParserRolesOrUsersSet::parseImpl(Pos & pos, ASTPtr & node, Expected & expec bool current_user = false; Strings except_names; bool except_current_user = false; - bool parsed_none = false; - if (!parseBeforeExcept(pos, expected, id_mode, allow_all, allow_any, allow_current_user, all, names, current_user, parsed_none)) + if (!parseBeforeExcept(pos, expected, id_mode, allow_all, allow_any, allow_current_user, all, names, current_user)) return false; - parseExceptAndAfterExcept(pos, expected, id_mode, allow_current_user, except_names, except_current_user, parsed_none); + parseExceptAndAfterExcept(pos, expected, id_mode, allow_current_user, except_names, except_current_user); if (all) names.clear(); @@ -146,7 +138,6 @@ bool ParserRolesOrUsersSet::parseImpl(Pos & pos, ASTPtr & node, Expected & expec result->allow_roles = allow_roles; result->id_mode = id_mode; result->use_keyword_any = all && allow_any && !allow_all; - result->none_role_parsed = parsed_none; node = result; return true; } diff --git a/tests/queries/0_stateless/01999_grant_with_replace.reference b/tests/queries/0_stateless/01999_grant_with_replace.reference index 3aabc8bd384..9e089a05e52 100644 --- a/tests/queries/0_stateless/01999_grant_with_replace.reference +++ b/tests/queries/0_stateless/01999_grant_with_replace.reference @@ -26,3 +26,11 @@ GRANT test_role_01999 TO test_user_01999 K GRANT SHOW ON db8.* TO test_user_01999 L +GRANT SELECT ON db9.tb3 TO test_user_01999 +M +GRANT SELECT ON db9.tb3 TO test_user_01999 +GRANT test_role_01999 TO test_user_01999 +N +GRANT SELECT ON db9.tb3 TO test_user_01999 +GRANT test_role_01999_1 TO test_user_01999 +O diff --git a/tests/queries/0_stateless/01999_grant_with_replace.sql b/tests/queries/0_stateless/01999_grant_with_replace.sql index 9c3f68fff7e..31a9187c0d2 100644 --- a/tests/queries/0_stateless/01999_grant_with_replace.sql +++ b/tests/queries/0_stateless/01999_grant_with_replace.sql @@ -54,7 +54,22 @@ SELECT 'K'; GRANT NONE TO test_user_01999 WITH REPLACE OPTION; SHOW GRANTS FOR test_user_01999; +SELECT 'L'; +GRANT NONE ON *.*, SELECT on db9.tb3 TO test_user_01999 WITH REPLACE OPTION; +SHOW GRANTS FOR test_user_01999; + +SELECT 'M'; +GRANT test_role_01999 to test_user_01999; +SHOW GRANTS FOR test_user_01999; + +SELECT 'N'; +DROP ROLE IF EXISTS test_role_01999_1; +CREATE role test_role_01999_1; +GRANT NONE, test_role_01999_1 TO test_user_01999 WITH REPLACE OPTION; +SHOW GRANTS FOR test_user_01999; + DROP USER IF EXISTS test_user_01999; DROP ROLE IF EXISTS test_role_01999; +DROP ROLE IF EXISTS test_role_01999_1; -SELECT 'L'; +SELECT 'O';