From 36a5b57ac4eaeac7f2356d6811acd1b0d1892523 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 8 Apr 2020 02:57:14 +0300 Subject: [PATCH 1/9] Use "CREATE SETTINGS PROFILE name SETTINGS INHERIT parent" instead of "CREATE SETTINGS PROFILE name SETTINGS PROFILE parent". --- ...InterpreterShowCreateAccessEntityQuery.cpp | 3 +++ src/Parsers/ASTCreateSettingsProfileQuery.h | 2 ++ src/Parsers/ASTSettingsProfileElement.cpp | 10 +++++++++- src/Parsers/ASTSettingsProfileElement.h | 3 +++ src/Parsers/IParser.h | 2 +- .../ParserCreateSettingsProfileQuery.cpp | 2 +- .../ParserCreateSettingsProfileQuery.h | 4 ++-- src/Parsers/ParserSettingsProfileElement.cpp | 19 +++++++++++++++---- src/Parsers/ParserSettingsProfileElement.h | 8 +++++++- .../test_disk_access_storage/test.py | 4 ++-- .../integration/test_settings_profile/test.py | 11 +++++++++++ 11 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp b/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp index d2f435106a8..e579ade11ca 100644 --- a/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp +++ b/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -101,6 +102,8 @@ namespace query->settings = profile.elements.toAST(); else query->settings = profile.elements.toASTWithNames(*manager); + if (query->settings) + query->settings->setUseInheritKeyword(true); } if (!profile.to_roles.empty()) diff --git a/src/Parsers/ASTCreateSettingsProfileQuery.h b/src/Parsers/ASTCreateSettingsProfileQuery.h index cc133397db4..eabe1ba441b 100644 --- a/src/Parsers/ASTCreateSettingsProfileQuery.h +++ b/src/Parsers/ASTCreateSettingsProfileQuery.h @@ -12,10 +12,12 @@ class ASTExtendedRoleSet; /** CREATE SETTINGS PROFILE [IF NOT EXISTS | OR REPLACE] name * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] + * [TO {role [,...] | ALL | ALL EXCEPT role [,...]}] * * ALTER SETTINGS PROFILE [IF EXISTS] name * [RENAME TO new_name] * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] + * [TO {role [,...] | ALL | ALL EXCEPT role [,...]}] */ class ASTCreateSettingsProfileQuery : public IAST, public ASTQueryWithOnCluster { diff --git a/src/Parsers/ASTSettingsProfileElement.cpp b/src/Parsers/ASTSettingsProfileElement.cpp index b3f4032d14c..24f1aa60813 100644 --- a/src/Parsers/ASTSettingsProfileElement.cpp +++ b/src/Parsers/ASTSettingsProfileElement.cpp @@ -25,7 +25,8 @@ void ASTSettingsProfileElement::formatImpl(const FormatSettings & settings, Form { if (!parent_profile.empty()) { - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "PROFILE " << (settings.hilite ? IAST::hilite_none : ""); + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << (use_inherit_keyword ? "INHERIT" : "PROFILE") << " " + << (settings.hilite ? IAST::hilite_none : ""); formatProfileNameOrID(parent_profile, id_mode, settings); return; } @@ -85,4 +86,11 @@ void ASTSettingsProfileElements::formatImpl(const FormatSettings & settings, For } } + +void ASTSettingsProfileElements::setUseInheritKeyword(bool use_inherit_keyword_) +{ + for (auto & element : elements) + element->use_inherit_keyword = use_inherit_keyword_; +} + } diff --git a/src/Parsers/ASTSettingsProfileElement.h b/src/Parsers/ASTSettingsProfileElement.h index 0470b51cf85..ee1ee28c383 100644 --- a/src/Parsers/ASTSettingsProfileElement.h +++ b/src/Parsers/ASTSettingsProfileElement.h @@ -19,6 +19,7 @@ public: Field max_value; std::optional readonly; bool id_mode = false; /// If true then `parent_profile` keeps UUID, not a name. + bool use_inherit_keyword = false; /// If true then this element is a part of ASTCreateSettingsProfileQuery. bool empty() const { return parent_profile.empty() && name.empty(); } @@ -41,5 +42,7 @@ public: String getID(char) const override { return "SettingsProfileElements"; } ASTPtr clone() const override { return std::make_shared(*this); } void formatImpl(const FormatSettings & settings, FormatState &, FormatStateStacked) const override; + + void setUseInheritKeyword(bool use_inherit_keyword_); }; } diff --git a/src/Parsers/IParser.h b/src/Parsers/IParser.h index 925140bd25e..5bfbf1ed476 100644 --- a/src/Parsers/IParser.h +++ b/src/Parsers/IParser.h @@ -126,7 +126,7 @@ public: return parse(pos, node, expected); } - virtual ~IParser() {} + virtual ~IParser() = default; }; using ParserPtr = std::unique_ptr; diff --git a/src/Parsers/ParserCreateSettingsProfileQuery.cpp b/src/Parsers/ParserCreateSettingsProfileQuery.cpp index 4d3ed2f6e63..5b33fed2fa0 100644 --- a/src/Parsers/ParserCreateSettingsProfileQuery.cpp +++ b/src/Parsers/ParserCreateSettingsProfileQuery.cpp @@ -33,7 +33,7 @@ namespace return false; ASTPtr new_settings_ast; - if (!ParserSettingsProfileElements{}.useIDMode(id_mode).parse(pos, new_settings_ast, expected)) + if (!ParserSettingsProfileElements{}.useIDMode(id_mode).enableInheritKeyword(true).parse(pos, new_settings_ast, expected)) return false; if (!settings) diff --git a/src/Parsers/ParserCreateSettingsProfileQuery.h b/src/Parsers/ParserCreateSettingsProfileQuery.h index 6797fc884fa..073a8ca75ae 100644 --- a/src/Parsers/ParserCreateSettingsProfileQuery.h +++ b/src/Parsers/ParserCreateSettingsProfileQuery.h @@ -7,11 +7,11 @@ namespace DB { /** Parses queries like * CREATE SETTINGS PROFILE [IF NOT EXISTS | OR REPLACE] name - * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] + * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | INHERIT 'profile_name'] [,...] * * ALTER SETTINGS PROFILE [IF EXISTS] name * [RENAME TO new_name] - * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] + * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | INHERIT 'profile_name'] [,...] */ class ParserCreateSettingsProfileQuery : public IParserBase { diff --git a/src/Parsers/ParserSettingsProfileElement.cpp b/src/Parsers/ParserSettingsProfileElement.cpp index 06fa58fde4e..31bc339f544 100644 --- a/src/Parsers/ParserSettingsProfileElement.cpp +++ b/src/Parsers/ParserSettingsProfileElement.cpp @@ -108,7 +108,8 @@ bool ParserSettingsProfileElement::parseImpl(Pos & pos, ASTPtr & node, Expected Field max_value; std::optional readonly; - if (ParserKeyword{"PROFILE"}.ignore(pos, expected)) + if (ParserKeyword{"PROFILE"}.ignore(pos, expected) || + (enable_inherit_keyword && ParserKeyword{"INHERIT"}.ignore(pos, expected))) { if (!parseProfileNameOrID(pos, expected, id_mode, parent_profile)) return false; @@ -120,9 +121,15 @@ bool ParserSettingsProfileElement::parseImpl(Pos & pos, ASTPtr & node, Expected return false; name = getIdentifierName(name_ast); + bool has_value_or_constraint = false; while (parseValue(pos, expected, value) || parseMinMaxValue(pos, expected, min_value, max_value) || parseReadonlyOrWritableKeyword(pos, expected, readonly)) - ; + { + has_value_or_constraint = true; + } + + if (!has_value_or_constraint) + return false; } auto result = std::make_shared(); @@ -133,6 +140,7 @@ bool ParserSettingsProfileElement::parseImpl(Pos & pos, ASTPtr & node, Expected result->max_value = std::move(max_value); result->readonly = readonly; result->id_mode = id_mode; + result->use_inherit_keyword = enable_inherit_keyword; node = result; return true; } @@ -142,12 +150,15 @@ bool ParserSettingsProfileElements::parseImpl(Pos & pos, ASTPtr & node, Expected { std::vector> elements; - if (!ParserKeyword{"NONE"}.ignore(pos, expected)) + if (ParserKeyword{"NONE"}.ignore(pos, expected)) + { + } + else { do { ASTPtr ast; - if (!ParserSettingsProfileElement{}.useIDMode(id_mode).parse(pos, ast, expected)) + if (!ParserSettingsProfileElement{}.useIDMode(id_mode).enableInheritKeyword(enable_inherit_keyword).parse(pos, ast, expected)) return false; auto element = typeid_cast>(ast); elements.push_back(std::move(element)); diff --git a/src/Parsers/ParserSettingsProfileElement.h b/src/Parsers/ParserSettingsProfileElement.h index ec8e1abb5b5..309c797e645 100644 --- a/src/Parsers/ParserSettingsProfileElement.h +++ b/src/Parsers/ParserSettingsProfileElement.h @@ -12,6 +12,7 @@ class ParserSettingsProfileElement : public IParserBase { public: ParserSettingsProfileElement & useIDMode(bool enable_) { id_mode = enable_; return *this; } + ParserSettingsProfileElement & enableInheritKeyword(bool enable_) { enable_inherit_keyword = enable_; return *this; } protected: const char * getName() const override { return "SettingsProfileElement"; } @@ -19,6 +20,7 @@ protected: private: bool id_mode = false; + bool enable_inherit_keyword = false; }; @@ -26,6 +28,7 @@ class ParserSettingsProfileElements : public IParserBase { public: ParserSettingsProfileElements & useIDMode(bool enable_) { id_mode = enable_; return *this; } + ParserSettingsProfileElements & enableInheritKeyword(bool enable_) { enable_inherit_keyword = enable_; return *this; } protected: const char * getName() const override { return "SettingsProfileElements"; } @@ -33,4 +36,7 @@ protected: private: bool id_mode = false; -};} + bool enable_inherit_keyword = false; +}; + +} diff --git a/tests/integration/test_disk_access_storage/test.py b/tests/integration/test_disk_access_storage/test.py index 1f6577b9dd1..019c1073205 100644 --- a/tests/integration/test_disk_access_storage/test.py +++ b/tests/integration/test_disk_access_storage/test.py @@ -47,7 +47,7 @@ def test_create(): assert instance.query("SHOW CREATE ROLE rx") == "CREATE ROLE rx SETTINGS PROFILE s1\n" assert instance.query("SHOW GRANTS FOR rx") == "" assert instance.query("SHOW CREATE SETTINGS PROFILE s1") == "CREATE SETTINGS PROFILE s1 SETTINGS max_memory_usage = 123456789 MIN 100000000 MAX 200000000\n" - assert instance.query("SHOW CREATE SETTINGS PROFILE s2") == "CREATE SETTINGS PROFILE s2 SETTINGS PROFILE s1 TO u2\n" + assert instance.query("SHOW CREATE SETTINGS PROFILE s2") == "CREATE SETTINGS PROFILE s2 SETTINGS INHERIT s1 TO u2\n" check() instance.restart_clickhouse() # Check persistency @@ -77,7 +77,7 @@ def test_alter(): assert instance.query("SHOW GRANTS FOR rx") == "GRANT SELECT ON mydb.* TO rx WITH GRANT OPTION\n" assert instance.query("SHOW GRANTS FOR ry") == "GRANT rx TO ry WITH ADMIN OPTION\n" assert instance.query("SHOW CREATE SETTINGS PROFILE s1") == "CREATE SETTINGS PROFILE s1 SETTINGS max_memory_usage = 987654321 READONLY\n" - assert instance.query("SHOW CREATE SETTINGS PROFILE s2") == "CREATE SETTINGS PROFILE s2 SETTINGS PROFILE s1 TO u2\n" + assert instance.query("SHOW CREATE SETTINGS PROFILE s2") == "CREATE SETTINGS PROFILE s2 SETTINGS INHERIT s1 TO u2\n" check() instance.restart_clickhouse() # Check persistency diff --git a/tests/integration/test_settings_profile/test.py b/tests/integration/test_settings_profile/test.py index 6866c6b3901..7ad3041b81e 100644 --- a/tests/integration/test_settings_profile/test.py +++ b/tests/integration/test_settings_profile/test.py @@ -31,22 +31,26 @@ def reset_after_test(): def test_settings_profile(): # Set settings and constraints via CREATE SETTINGS PROFILE ... TO user instance.query("CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000 TO robin") + assert instance.query("SHOW CREATE SETTINGS PROFILE xyz") == "CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000 TO robin\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "100000001\n" assert "Setting max_memory_usage shouldn't be less than 90000000" in instance.query_and_get_error("SET max_memory_usage = 80000000", user="robin") assert "Setting max_memory_usage shouldn't be greater than 110000000" in instance.query_and_get_error("SET max_memory_usage = 120000000", user="robin") instance.query("ALTER SETTINGS PROFILE xyz TO NONE") + assert instance.query("SHOW CREATE SETTINGS PROFILE xyz") == "CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "10000000000\n" instance.query("SET max_memory_usage = 80000000", user="robin") instance.query("SET max_memory_usage = 120000000", user="robin") # Set settings and constraints via CREATE USER ... SETTINGS PROFILE instance.query("ALTER USER robin SETTINGS PROFILE xyz") + assert instance.query("SHOW CREATE USER robin") == "CREATE USER robin SETTINGS PROFILE xyz\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "100000001\n" assert "Setting max_memory_usage shouldn't be less than 90000000" in instance.query_and_get_error("SET max_memory_usage = 80000000", user="robin") assert "Setting max_memory_usage shouldn't be greater than 110000000" in instance.query_and_get_error("SET max_memory_usage = 120000000", user="robin") instance.query("ALTER USER robin SETTINGS NONE") + assert instance.query("SHOW CREATE USER robin") == "CREATE USER robin\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "10000000000\n" instance.query("SET max_memory_usage = 80000000", user="robin") instance.query("SET max_memory_usage = 120000000", user="robin") @@ -57,6 +61,8 @@ def test_settings_profile_from_granted_role(): instance.query("CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000") instance.query("CREATE ROLE worker SETTINGS PROFILE xyz") instance.query("GRANT worker TO robin") + assert instance.query("SHOW CREATE SETTINGS PROFILE xyz") == "CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000\n" + assert instance.query("SHOW CREATE ROLE worker") == "CREATE ROLE worker SETTINGS PROFILE xyz\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "100000001\n" assert "Setting max_memory_usage shouldn't be less than 90000000" in instance.query_and_get_error("SET max_memory_usage = 80000000", user="robin") assert "Setting max_memory_usage shouldn't be greater than 110000000" in instance.query_and_get_error("SET max_memory_usage = 120000000", user="robin") @@ -68,17 +74,20 @@ def test_settings_profile_from_granted_role(): instance.query("ALTER ROLE worker SETTINGS NONE") instance.query("GRANT worker TO robin") + assert instance.query("SHOW CREATE ROLE worker") == "CREATE ROLE worker\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "10000000000\n" instance.query("SET max_memory_usage = 80000000", user="robin") instance.query("SET max_memory_usage = 120000000", user="robin") # Set settings and constraints via CREATE SETTINGS PROFILE ... TO granted role instance.query("ALTER SETTINGS PROFILE xyz TO worker") + assert instance.query("SHOW CREATE SETTINGS PROFILE xyz") == "CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000 TO worker\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "100000001\n" assert "Setting max_memory_usage shouldn't be less than 90000000" in instance.query_and_get_error("SET max_memory_usage = 80000000", user="robin") assert "Setting max_memory_usage shouldn't be greater than 110000000" in instance.query_and_get_error("SET max_memory_usage = 120000000", user="robin") instance.query("ALTER SETTINGS PROFILE xyz TO NONE") + assert instance.query("SHOW CREATE SETTINGS PROFILE xyz") == "CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000001 MIN 90000000 MAX 110000000\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "10000000000\n" instance.query("SET max_memory_usage = 80000000", user="robin") instance.query("SET max_memory_usage = 120000000", user="robin") @@ -87,6 +96,8 @@ def test_settings_profile_from_granted_role(): def test_inheritance_of_settings_profile(): instance.query("CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000002 READONLY") instance.query("CREATE SETTINGS PROFILE alpha SETTINGS PROFILE xyz TO robin") + assert instance.query("SHOW CREATE SETTINGS PROFILE xyz") == "CREATE SETTINGS PROFILE xyz SETTINGS max_memory_usage = 100000002 READONLY\n" + assert instance.query("SHOW CREATE SETTINGS PROFILE alpha") == "CREATE SETTINGS PROFILE alpha SETTINGS INHERIT xyz TO robin\n" assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "100000002\n" assert "Setting max_memory_usage should not be changed" in instance.query_and_get_error("SET max_memory_usage = 80000000", user="robin") From c97d12a19c96f3857864c6f00a75d0c0ede2c341 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 8 Apr 2020 03:50:27 +0300 Subject: [PATCH 2/9] Enable access management by default for all integration tests. --- .../0_common_instance_users.xml} | 0 tests/integration/helpers/cluster.py | 1 + .../test_allowed_client_hosts/configs/users.xml | 13 ------------- .../configs/users.d/access_management.xml | 7 ------- tests/integration/test_authentication/test.py | 2 +- .../configs/users.d/access_management.xml | 7 ------- .../test_disk_access_storage/test.py | 2 +- .../configs/users.d/access_management.xml | 7 ------- tests/integration/test_grant_and_revoke/test.py | 2 +- ...access_management.xml => assign_myquota.xml} | 2 +- .../configs/users.d/drop_default_quota.xml | 5 +++++ tests/integration/test_quota/configs/users.xml | 17 ----------------- .../configs/users.d/access_management.xml | 7 ------- .../configs/{config.d => }/remote_servers.xml | 0 .../configs/users.d/access_management.xml | 7 ------- .../test.py | 6 +++--- .../configs/users.d/access_management.xml | 7 ------- tests/integration/test_settings_profile/test.py | 2 +- 18 files changed, 14 insertions(+), 80 deletions(-) rename tests/integration/{test_access_control_on_cluster/configs/users.d/access_management.xml => helpers/0_common_instance_users.xml} (100%) delete mode 100644 tests/integration/test_allowed_client_hosts/configs/users.xml delete mode 100644 tests/integration/test_authentication/configs/users.d/access_management.xml delete mode 100644 tests/integration/test_disk_access_storage/configs/users.d/access_management.xml delete mode 100644 tests/integration/test_grant_and_revoke/configs/users.d/access_management.xml rename tests/integration/test_quota/configs/users.d/{access_management.xml => assign_myquota.xml} (60%) create mode 100644 tests/integration/test_quota/configs/users.d/drop_default_quota.xml delete mode 100644 tests/integration/test_quota/configs/users.xml delete mode 100644 tests/integration/test_row_policy/configs/users.d/access_management.xml rename tests/integration/test_settings_constraints_distributed/configs/{config.d => }/remote_servers.xml (100%) delete mode 100644 tests/integration/test_settings_constraints_distributed/configs/users.d/access_management.xml delete mode 100644 tests/integration/test_settings_profile/configs/users.d/access_management.xml diff --git a/tests/integration/test_access_control_on_cluster/configs/users.d/access_management.xml b/tests/integration/helpers/0_common_instance_users.xml similarity index 100% rename from tests/integration/test_access_control_on_cluster/configs/users.d/access_management.xml rename to tests/integration/helpers/0_common_instance_users.xml diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index 5dc93cb338a..69f8206b2c1 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -923,6 +923,7 @@ class ClickHouseInstance: # The file is named with 0_ prefix to be processed before other configuration overloads. shutil.copy(p.join(HELPERS_DIR, '0_common_instance_config.xml'), self.config_d_dir) + shutil.copy(p.join(HELPERS_DIR, '0_common_instance_users.xml'), users_d_dir) # Generate and write macros file macros = self.macros.copy() diff --git a/tests/integration/test_allowed_client_hosts/configs/users.xml b/tests/integration/test_allowed_client_hosts/configs/users.xml deleted file mode 100644 index 3142ec5355a..00000000000 --- a/tests/integration/test_allowed_client_hosts/configs/users.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - - - default - - - - diff --git a/tests/integration/test_authentication/configs/users.d/access_management.xml b/tests/integration/test_authentication/configs/users.d/access_management.xml deleted file mode 100644 index 7e799cb7b10..00000000000 --- a/tests/integration/test_authentication/configs/users.d/access_management.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1 - - - diff --git a/tests/integration/test_authentication/test.py b/tests/integration/test_authentication/test.py index b7ffd1ed35b..483b59813e5 100644 --- a/tests/integration/test_authentication/test.py +++ b/tests/integration/test_authentication/test.py @@ -2,7 +2,7 @@ import pytest from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) -instance = cluster.add_instance('instance', config_dir="configs") +instance = cluster.add_instance('instance') @pytest.fixture(scope="module", autouse=True) diff --git a/tests/integration/test_disk_access_storage/configs/users.d/access_management.xml b/tests/integration/test_disk_access_storage/configs/users.d/access_management.xml deleted file mode 100644 index 7e799cb7b10..00000000000 --- a/tests/integration/test_disk_access_storage/configs/users.d/access_management.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1 - - - diff --git a/tests/integration/test_disk_access_storage/test.py b/tests/integration/test_disk_access_storage/test.py index 019c1073205..0db0e21afef 100644 --- a/tests/integration/test_disk_access_storage/test.py +++ b/tests/integration/test_disk_access_storage/test.py @@ -2,7 +2,7 @@ import pytest from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) -instance = cluster.add_instance('instance', config_dir='configs', stay_alive=True) +instance = cluster.add_instance('instance', stay_alive=True) @pytest.fixture(scope="module", autouse=True) diff --git a/tests/integration/test_grant_and_revoke/configs/users.d/access_management.xml b/tests/integration/test_grant_and_revoke/configs/users.d/access_management.xml deleted file mode 100644 index 7e799cb7b10..00000000000 --- a/tests/integration/test_grant_and_revoke/configs/users.d/access_management.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1 - - - diff --git a/tests/integration/test_grant_and_revoke/test.py b/tests/integration/test_grant_and_revoke/test.py index 25e0e9882de..6f4b0be5325 100644 --- a/tests/integration/test_grant_and_revoke/test.py +++ b/tests/integration/test_grant_and_revoke/test.py @@ -3,7 +3,7 @@ from helpers.cluster import ClickHouseCluster import re cluster = ClickHouseCluster(__file__) -instance = cluster.add_instance('instance', config_dir="configs") +instance = cluster.add_instance('instance') @pytest.fixture(scope="module", autouse=True) diff --git a/tests/integration/test_quota/configs/users.d/access_management.xml b/tests/integration/test_quota/configs/users.d/assign_myquota.xml similarity index 60% rename from tests/integration/test_quota/configs/users.d/access_management.xml rename to tests/integration/test_quota/configs/users.d/assign_myquota.xml index 7e799cb7b10..8b98ade8aeb 100644 --- a/tests/integration/test_quota/configs/users.d/access_management.xml +++ b/tests/integration/test_quota/configs/users.d/assign_myquota.xml @@ -1,7 +1,7 @@ - 1 + myQuota diff --git a/tests/integration/test_quota/configs/users.d/drop_default_quota.xml b/tests/integration/test_quota/configs/users.d/drop_default_quota.xml new file mode 100644 index 00000000000..5f53ecf5f49 --- /dev/null +++ b/tests/integration/test_quota/configs/users.d/drop_default_quota.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/integration/test_quota/configs/users.xml b/tests/integration/test_quota/configs/users.xml deleted file mode 100644 index 4412345a731..00000000000 --- a/tests/integration/test_quota/configs/users.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - ::/0 - - default - myQuota - - - diff --git a/tests/integration/test_row_policy/configs/users.d/access_management.xml b/tests/integration/test_row_policy/configs/users.d/access_management.xml deleted file mode 100644 index 7e799cb7b10..00000000000 --- a/tests/integration/test_row_policy/configs/users.d/access_management.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1 - - - diff --git a/tests/integration/test_settings_constraints_distributed/configs/config.d/remote_servers.xml b/tests/integration/test_settings_constraints_distributed/configs/remote_servers.xml similarity index 100% rename from tests/integration/test_settings_constraints_distributed/configs/config.d/remote_servers.xml rename to tests/integration/test_settings_constraints_distributed/configs/remote_servers.xml diff --git a/tests/integration/test_settings_constraints_distributed/configs/users.d/access_management.xml b/tests/integration/test_settings_constraints_distributed/configs/users.d/access_management.xml deleted file mode 100644 index 7e799cb7b10..00000000000 --- a/tests/integration/test_settings_constraints_distributed/configs/users.d/access_management.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1 - - - diff --git a/tests/integration/test_settings_constraints_distributed/test.py b/tests/integration/test_settings_constraints_distributed/test.py index a58c037a2fc..51999902e7d 100644 --- a/tests/integration/test_settings_constraints_distributed/test.py +++ b/tests/integration/test_settings_constraints_distributed/test.py @@ -8,9 +8,9 @@ from helpers.test_tools import assert_eq_with_retry cluster = ClickHouseCluster(__file__) -node1 = cluster.add_instance('node1', config_dir="configs") -node2 = cluster.add_instance('node2', config_dir="configs") -distributed = cluster.add_instance('distributed', config_dir="configs") +node1 = cluster.add_instance('node1') +node2 = cluster.add_instance('node2') +distributed = cluster.add_instance('distributed', main_configs=["configs/remote_servers.xml"]) @pytest.fixture(scope="module") diff --git a/tests/integration/test_settings_profile/configs/users.d/access_management.xml b/tests/integration/test_settings_profile/configs/users.d/access_management.xml deleted file mode 100644 index 7e799cb7b10..00000000000 --- a/tests/integration/test_settings_profile/configs/users.d/access_management.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - 1 - - - diff --git a/tests/integration/test_settings_profile/test.py b/tests/integration/test_settings_profile/test.py index 7ad3041b81e..8b9d023d56f 100644 --- a/tests/integration/test_settings_profile/test.py +++ b/tests/integration/test_settings_profile/test.py @@ -2,7 +2,7 @@ import pytest from helpers.cluster import ClickHouseCluster cluster = ClickHouseCluster(__file__) -instance = cluster.add_instance('instance', config_dir="configs") +instance = cluster.add_instance('instance') @pytest.fixture(scope="module", autouse=True) From 23ac1ee87c87ae20152bf3593284203f01bacfdc Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 8 Apr 2020 04:35:15 +0300 Subject: [PATCH 3/9] readonly user now can execute SHOW CREATE for access entities. --- src/Access/ContextAccess.cpp | 3 ++- .../__init__.py | 0 .../configs/users.d/extra_users.xml | 13 ++++++++++ .../test_enabling_access_management/test.py | 24 +++++++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_enabling_access_management/__init__.py create mode 100644 tests/integration/test_enabling_access_management/configs/users.d/extra_users.xml create mode 100644 tests/integration/test_enabling_access_management/test.py diff --git a/src/Access/ContextAccess.cpp b/src/Access/ContextAccess.cpp index 14775f7a4de..cf788a0a63e 100644 --- a/src/Access/ContextAccess.cpp +++ b/src/Access/ContextAccess.cpp @@ -408,9 +408,10 @@ boost::shared_ptr ContextAccess::calculateResultAccess(bool static const AccessFlags dictionary_ddl = AccessType::CREATE_DICTIONARY | AccessType::DROP_DICTIONARY; static const AccessFlags table_and_dictionary_ddl = table_ddl | dictionary_ddl; static const AccessFlags write_table_access = AccessType::INSERT | AccessType::OPTIMIZE; + static const AccessFlags write_dcl_access = AccessType::ACCESS_MANAGEMENT - AccessType::SHOW_ACCESS; if (readonly_) - merged_access->revoke(write_table_access | table_and_dictionary_ddl | AccessType::SYSTEM | AccessType::KILL_QUERY | AccessType::ACCESS_MANAGEMENT); + merged_access->revoke(write_table_access | table_and_dictionary_ddl | write_dcl_access | AccessType::SYSTEM | AccessType::KILL_QUERY); if (readonly_ == 1) { diff --git a/tests/integration/test_enabling_access_management/__init__.py b/tests/integration/test_enabling_access_management/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_enabling_access_management/configs/users.d/extra_users.xml b/tests/integration/test_enabling_access_management/configs/users.d/extra_users.xml new file mode 100644 index 00000000000..7d87a29a915 --- /dev/null +++ b/tests/integration/test_enabling_access_management/configs/users.d/extra_users.xml @@ -0,0 +1,13 @@ + + + + + readonly + 1 + + + + default + + + diff --git a/tests/integration/test_enabling_access_management/test.py b/tests/integration/test_enabling_access_management/test.py new file mode 100644 index 00000000000..abb8cd6c07a --- /dev/null +++ b/tests/integration/test_enabling_access_management/test.py @@ -0,0 +1,24 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +instance = cluster.add_instance('instance', config_dir="configs") + +@pytest.fixture(scope="module", autouse=True) +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +def test_enabling_access_management(): + instance.query("CREATE USER Alex", user='default') + assert instance.query("SHOW CREATE USER Alex", user='default') == "CREATE USER Alex\n" + assert instance.query("SHOW CREATE USER Alex", user='readonly') == "CREATE USER Alex\n" + assert "Not enough privileges" in instance.query_and_get_error("SHOW CREATE USER Alex", user='xyz') + + assert "Cannot execute query in readonly mode" in instance.query_and_get_error("CREATE USER Robin", user='readonly') + assert "Not enough privileges" in instance.query_and_get_error("CREATE USER Robin", user='xyz') From d548c7e381e412c8f7d4e2d733bf92765699ac0a Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 8 Apr 2020 06:09:40 +0300 Subject: [PATCH 4/9] Simplify DCL for creating quotas. --- .../InterpreterCreateQuotaQuery.cpp | 4 +- ...InterpreterShowCreateAccessEntityQuery.cpp | 2 +- src/Parsers/ASTCreateQuotaQuery.cpp | 25 +++++---- src/Parsers/ASTCreateQuotaQuery.h | 11 ++-- src/Parsers/ParserCreateQuotaQuery.cpp | 53 ++++++++----------- src/Parsers/ParserCreateQuotaQuery.h | 9 ++-- .../test_disk_access_storage/test.py | 4 +- tests/integration/test_quota/test.py | 22 ++++---- .../0_stateless/01033_quota_dcl.reference | 2 +- 9 files changed, 61 insertions(+), 71 deletions(-) diff --git a/src/Interpreters/InterpreterCreateQuotaQuery.cpp b/src/Interpreters/InterpreterCreateQuotaQuery.cpp index 13e772965ff..80987993c96 100644 --- a/src/Interpreters/InterpreterCreateQuotaQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuotaQuery.cpp @@ -34,7 +34,7 @@ void updateQuotaFromQueryImpl(Quota & quota, const ASTCreateQuotaQuery & query, auto duration = query_limits.duration; auto it = boost::range::find_if(quota_all_limits, [&](const Quota::Limits & x) { return x.duration == duration; }); - if (query_limits.unset_tracking) + if (query_limits.drop) { if (it != quota_all_limits.end()) quota_all_limits.erase(it); @@ -59,6 +59,8 @@ void updateQuotaFromQueryImpl(Quota & quota, const ASTCreateQuotaQuery & query, { if (query_limits.max[resource_type]) quota_limits.max[resource_type] = *query_limits.max[resource_type]; + else + quota_limits.max[resource_type] = Quota::UNLIMITED; } } diff --git a/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp b/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp index e579ade11ca..4c2dcc19a88 100644 --- a/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp +++ b/src/Interpreters/InterpreterShowCreateAccessEntityQuery.cpp @@ -136,7 +136,7 @@ namespace create_query_limits.duration = limits.duration; create_query_limits.randomize_interval = limits.randomize_interval; for (auto resource_type : ext::range(Quota::MAX_RESOURCE_TYPE)) - if (limits.max[resource_type]) + if (limits.max[resource_type] != Quota::UNLIMITED) create_query_limits.max[resource_type] = limits.max[resource_type]; query->all_limits.push_back(create_query_limits); } diff --git a/src/Parsers/ASTCreateQuotaQuery.cpp b/src/Parsers/ASTCreateQuotaQuery.cpp index 8fa0dbb0d31..cd064756fb6 100644 --- a/src/Parsers/ASTCreateQuotaQuery.cpp +++ b/src/Parsers/ASTCreateQuotaQuery.cpp @@ -28,16 +28,17 @@ namespace } - void formatLimit(ResourceType resource_type, ResourceAmount max, const IAST::FormatSettings & settings) + void formatLimit(ResourceType resource_type, ResourceAmount max, bool first, const IAST::FormatSettings & settings) { - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " MAX " << Quota::resourceTypeToKeyword(resource_type) - << (settings.hilite ? IAST::hilite_none : ""); + if (first) + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " MAX" << (settings.hilite ? IAST::hilite_none : ""); + else + settings.ostr << ","; - settings.ostr << (settings.hilite ? IAST::hilite_operator : "") << " = " << (settings.hilite ? IAST::hilite_none : ""); + settings.ostr << " " << (settings.hilite ? IAST::hilite_keyword : "") << Quota::resourceTypeToKeyword(resource_type) + << (settings.hilite ? IAST::hilite_none : "") << " "; - if (max == Quota::UNLIMITED) - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "ANY" << (settings.hilite ? IAST::hilite_none : ""); - else if (resource_type == Quota::EXECUTION_TIME) + if (resource_type == Quota::EXECUTION_TIME) settings.ostr << Quota::executionTimeToSeconds(max); else settings.ostr << max; @@ -59,9 +60,9 @@ namespace << interval_kind.toKeyword() << (settings.hilite ? IAST::hilite_none : ""); - if (limits.unset_tracking) + if (limits.drop) { - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " UNSET TRACKING" << (settings.hilite ? IAST::hilite_none : ""); + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " NO LIMITS" << (settings.hilite ? IAST::hilite_none : ""); } else { @@ -70,14 +71,12 @@ namespace { if (limits.max[resource_type]) { - if (limit_found) - settings.ostr << ","; + formatLimit(resource_type, *limits.max[resource_type], !limit_found, settings); limit_found = true; - formatLimit(resource_type, *limits.max[resource_type], settings); } } if (!limit_found) - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " TRACKING" << (settings.hilite ? IAST::hilite_none : ""); + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " TRACKING ONLY" << (settings.hilite ? IAST::hilite_none : ""); } } diff --git a/src/Parsers/ASTCreateQuotaQuery.h b/src/Parsers/ASTCreateQuotaQuery.h index 09ceaea9825..70f8cba6de0 100644 --- a/src/Parsers/ASTCreateQuotaQuery.h +++ b/src/Parsers/ASTCreateQuotaQuery.h @@ -13,17 +13,16 @@ class ASTExtendedRoleSet; /** CREATE QUOTA [IF NOT EXISTS | OR REPLACE] name * [KEYED BY {'none' | 'user name' | 'ip address' | 'client key' | 'client key or user name' | 'client key or ip address'}] * [FOR [RANDOMIZED] INTERVAL number {SECOND | MINUTE | HOUR | DAY} - * {[SET] MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = {number | ANY} } [,...] | - * [SET] TRACKING} [,...]] + * {MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = number} [,...] | + * NO LIMITS | TRACKING ONLY} [,...]] * [TO {role [,...] | ALL | ALL EXCEPT role [,...]}] * * ALTER QUOTA [IF EXISTS] name * [RENAME TO new_name] * [KEYED BY {'none' | 'user name' | 'ip address' | 'client key' | 'client key or user name' | 'client key or ip address'}] * [FOR [RANDOMIZED] INTERVAL number {SECOND | MINUTE | HOUR | DAY} - * {[SET] MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = {number | ANY} } [,...] | - * [SET] TRACKING | - * UNSET TRACKING} [,...]] + * {MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = number} [,...] | + * NO LIMITS | TRACKING ONLY} [,...]] * [TO {role [,...] | ALL | ALL EXCEPT role [,...]}] */ class ASTCreateQuotaQuery : public IAST, public ASTQueryWithOnCluster @@ -48,7 +47,7 @@ public: struct Limits { std::optional max[MAX_RESOURCE_TYPE]; - bool unset_tracking = false; + bool drop = false; std::chrono::seconds duration = std::chrono::seconds::zero(); bool randomize_interval = false; }; diff --git a/src/Parsers/ParserCreateQuotaQuery.cpp b/src/Parsers/ParserCreateQuotaQuery.cpp index 66e72ee4968..8bbd2127922 100644 --- a/src/Parsers/ParserCreateQuotaQuery.cpp +++ b/src/Parsers/ParserCreateQuotaQuery.cpp @@ -63,12 +63,22 @@ namespace }); } - bool parseLimit(IParserBase::Pos & pos, Expected & expected, ResourceType & resource_type, ResourceAmount & max) + bool parseLimit(IParserBase::Pos & pos, Expected & expected, bool first, ResourceType & resource_type, ResourceAmount & max) { return IParserBase::wrapParseImpl(pos, [&] { - if (!ParserKeyword{"MAX"}.ignore(pos, expected)) - return false; + if (first) + { + if (!ParserKeyword{"MAX"}.ignore(pos, expected)) + return false; + } + else + { + if (!ParserToken{TokenType::Comma}.ignore(pos, expected)) + return false; + + ParserKeyword{"MAX"}.ignore(pos, expected); + } bool resource_type_set = false; for (auto rt : ext::range_with_static_cast(Quota::MAX_RESOURCE_TYPE)) @@ -83,9 +93,6 @@ namespace if (!resource_type_set) return false; - if (!ParserToken{TokenType::Equals}.ignore(pos, expected)) - return false; - ASTPtr max_ast; if (ParserNumber{}.parse(pos, max_ast, expected)) { @@ -95,10 +102,6 @@ namespace else max = applyVisitor(FieldVisitorConvertToNumber(), max_field); } - else if (ParserKeyword{"ANY"}.ignore(pos, expected)) - { - max = Quota::UNLIMITED; - } else return false; @@ -106,18 +109,7 @@ namespace }); } - bool parseCommaAndLimit(IParserBase::Pos & pos, Expected & expected, ResourceType & resource_type, ResourceAmount & max) - { - return IParserBase::wrapParseImpl(pos, [&] - { - if (!ParserToken{TokenType::Comma}.ignore(pos, expected)) - return false; - - return parseLimit(pos, expected, resource_type, max); - }); - } - - bool parseLimits(IParserBase::Pos & pos, Expected & expected, bool alter, ASTCreateQuotaQuery::Limits & limits) + bool parseLimits(IParserBase::Pos & pos, Expected & expected, ASTCreateQuotaQuery::Limits & limits) { return IParserBase::wrapParseImpl(pos, [&] { @@ -142,23 +134,22 @@ namespace new_limits.duration = std::chrono::seconds(static_cast(num_intervals * interval_kind.toAvgSeconds())); - if (alter && ParserKeyword{"UNSET TRACKING"}.ignore(pos, expected)) + if (ParserKeyword{"NO LIMITS"}.ignore(pos, expected)) { - new_limits.unset_tracking = true; + new_limits.drop = true; } - else if (ParserKeyword{"SET TRACKING"}.ignore(pos, expected) || ParserKeyword{"TRACKING"}.ignore(pos, expected)) + else if (ParserKeyword{"TRACKING ONLY"}.ignore(pos, expected)) { } else { - ParserKeyword{"SET"}.ignore(pos, expected); ResourceType resource_type; ResourceAmount max; - if (!parseLimit(pos, expected, resource_type, max)) + if (!parseLimit(pos, expected, true, resource_type, max)) return false; new_limits.max[resource_type] = max; - while (parseCommaAndLimit(pos, expected, resource_type, max)) + while (parseLimit(pos, expected, false, resource_type, max)) new_limits.max[resource_type] = max; } @@ -167,7 +158,7 @@ namespace }); } - bool parseAllLimits(IParserBase::Pos & pos, Expected & expected, bool alter, std::vector & all_limits) + bool parseAllLimits(IParserBase::Pos & pos, Expected & expected, std::vector & all_limits) { return IParserBase::wrapParseImpl(pos, [&] { @@ -175,7 +166,7 @@ namespace do { ASTCreateQuotaQuery::Limits limits; - if (!parseLimits(pos, expected, alter, limits)) + if (!parseLimits(pos, expected, limits)) { all_limits.resize(old_size); return false; @@ -257,7 +248,7 @@ bool ParserCreateQuotaQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe if (!key_type && parseKeyType(pos, expected, key_type)) continue; - if (parseAllLimits(pos, expected, alter, all_limits)) + if (parseAllLimits(pos, expected, all_limits)) continue; break; diff --git a/src/Parsers/ParserCreateQuotaQuery.h b/src/Parsers/ParserCreateQuotaQuery.h index 18e6ef6f9f7..786c8292b15 100644 --- a/src/Parsers/ParserCreateQuotaQuery.h +++ b/src/Parsers/ParserCreateQuotaQuery.h @@ -9,17 +9,16 @@ namespace DB * CREATE QUOTA [IF NOT EXISTS | OR REPLACE] name * [KEYED BY {'none' | 'user name' | 'ip address' | 'client key' | 'client key or user name' | 'client key or ip address'}] * [FOR [RANDOMIZED] INTERVAL number {SECOND | MINUTE | HOUR | DAY} - * {[SET] MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = {number | ANY} } [,...] | - * [SET] TRACKING} [,...]] + * {MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = number} [,...] | + * NO LIMITS | TRACKING ONLY} [,...]] * [TO {role [,...] | ALL | ALL EXCEPT role [,...]}] * * ALTER QUOTA [IF EXISTS] name * [RENAME TO new_name] * [KEYED BY {'none' | 'user name' | 'ip address' | 'client key' | 'client key or user name' | 'client key or ip address'}] * [FOR [RANDOMIZED] INTERVAL number {SECOND | MINUTE | HOUR | DAY} - * {[SET] MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = {number | ANY} } [,...] | - * [SET] TRACKING | - * UNSET TRACKING} [,...]] + * {MAX {{QUERIES | ERRORS | RESULT ROWS | RESULT BYTES | READ ROWS | READ BYTES | EXECUTION TIME} = number} } [,...] | + * NO LIMITS | TRACKING ONLY} [,...]] * [TO {role [,...] | ALL | ALL EXCEPT role [,...]}] */ class ParserCreateQuotaQuery : public IParserBase diff --git a/tests/integration/test_disk_access_storage/test.py b/tests/integration/test_disk_access_storage/test.py index 0db0e21afef..babceee7c76 100644 --- a/tests/integration/test_disk_access_storage/test.py +++ b/tests/integration/test_disk_access_storage/test.py @@ -22,7 +22,7 @@ def create_entities(): instance.query("CREATE USER u2 IDENTIFIED BY 'qwerty' HOST LOCAL DEFAULT ROLE rx") instance.query("CREATE SETTINGS PROFILE s2 SETTINGS PROFILE s1 TO u2") instance.query("CREATE ROW POLICY p ON mydb.mytable FOR SELECT USING a<1000 TO u1, u2") - instance.query("CREATE QUOTA q FOR INTERVAL 1 HOUR SET MAX QUERIES = 100 TO ALL EXCEPT rx") + instance.query("CREATE QUOTA q FOR INTERVAL 1 HOUR MAX QUERIES 100 TO ALL EXCEPT rx") @pytest.fixture(autouse=True) @@ -41,7 +41,7 @@ def test_create(): assert instance.query("SHOW CREATE USER u1") == "CREATE USER u1 SETTINGS PROFILE s1\n" assert instance.query("SHOW CREATE USER u2") == "CREATE USER u2 HOST LOCAL DEFAULT ROLE rx\n" assert instance.query("SHOW CREATE ROW POLICY p ON mydb.mytable") == "CREATE ROW POLICY p ON mydb.mytable FOR SELECT USING a < 1000 TO u1, u2\n" - assert instance.query("SHOW CREATE QUOTA q") == "CREATE QUOTA q KEYED BY \\'none\\' FOR INTERVAL 1 HOUR MAX QUERIES = 100 TO ALL EXCEPT rx\n" + assert instance.query("SHOW CREATE QUOTA q") == "CREATE QUOTA q KEYED BY \\'none\\' FOR INTERVAL 1 HOUR MAX QUERIES 100 TO ALL EXCEPT rx\n" assert instance.query("SHOW GRANTS FOR u1") == "" assert instance.query("SHOW GRANTS FOR u2") == "GRANT rx TO u2\n" assert instance.query("SHOW CREATE ROLE rx") == "CREATE ROLE rx SETTINGS PROFILE s1\n" diff --git a/tests/integration/test_quota/test.py b/tests/integration/test_quota/test.py index 85d2ded16c1..ae68a34a03e 100644 --- a/tests/integration/test_quota/test.py +++ b/tests/integration/test_quota/test.py @@ -180,7 +180,7 @@ def test_reload_users_xml_by_timer(): def test_dcl_introspection(): assert instance.query("SHOW QUOTAS") == "myQuota\n" - assert instance.query("SHOW CREATE QUOTA myQuota") == "CREATE QUOTA myQuota KEYED BY \\'user name\\' FOR INTERVAL 1 YEAR MAX QUERIES = 1000, MAX READ ROWS = 1000 TO default\n" + assert instance.query("SHOW CREATE QUOTA myQuota") == "CREATE QUOTA myQuota KEYED BY \\'user name\\' FOR INTERVAL 1 YEAR MAX QUERIES 1000, READ ROWS 1000 TO default\n" expected_usage = "myQuota key=\\\\'default\\\\' interval=\[.*\] queries=0/1000 errors=0 result_rows=0 result_bytes=0 read_rows=0/1000 read_bytes=0 execution_time=0" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE CURRENT")) @@ -193,7 +193,7 @@ def test_dcl_introspection(): # Add interval. copy_quota_xml('two_intervals.xml') assert instance.query("SHOW QUOTAS") == "myQuota\n" - assert instance.query("SHOW CREATE QUOTA myQuota") == "CREATE QUOTA myQuota KEYED BY \\'user name\\' FOR INTERVAL 1 YEAR MAX QUERIES = 1000, MAX READ ROWS = 1000, FOR RANDOMIZED INTERVAL 2 YEAR MAX RESULT BYTES = 30000, MAX READ BYTES = 20000, MAX EXECUTION TIME = 120 TO default\n" + assert instance.query("SHOW CREATE QUOTA myQuota") == "CREATE QUOTA myQuota KEYED BY \\'user name\\' FOR INTERVAL 1 YEAR MAX QUERIES 1000, READ ROWS 1000, FOR RANDOMIZED INTERVAL 2 YEAR MAX RESULT BYTES 30000, READ BYTES 20000, EXECUTION TIME 120 TO default\n" expected_usage = "myQuota key=\\\\'default\\\\' interval=\[.*\] queries=1/1000 errors=0 result_rows=50 result_bytes=200 read_rows=50/1000 read_bytes=200 execution_time=.*\n"\ "myQuota key=\\\\'default\\\\' interval=\[.*\] queries=0 errors=0 result_rows=0 result_bytes=0/30000 read_rows=0 read_bytes=0/20000 execution_time=0/120" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) @@ -201,8 +201,8 @@ def test_dcl_introspection(): # Drop interval, add quota. copy_quota_xml('two_quotas.xml') assert instance.query("SHOW QUOTAS") == "myQuota\nmyQuota2\n" - assert instance.query("SHOW CREATE QUOTA myQuota") == "CREATE QUOTA myQuota KEYED BY \\'user name\\' FOR INTERVAL 1 YEAR MAX QUERIES = 1000, MAX READ ROWS = 1000 TO default\n" - assert instance.query("SHOW CREATE QUOTA myQuota2") == "CREATE QUOTA myQuota2 KEYED BY \\'client key or user name\\' FOR RANDOMIZED INTERVAL 1 HOUR MAX RESULT ROWS = 4000, MAX RESULT BYTES = 400000, MAX READ ROWS = 4000, MAX READ BYTES = 400000, MAX EXECUTION TIME = 60, FOR INTERVAL 1 MONTH MAX EXECUTION TIME = 1800\n" + assert instance.query("SHOW CREATE QUOTA myQuota") == "CREATE QUOTA myQuota KEYED BY \\'user name\\' FOR INTERVAL 1 YEAR MAX QUERIES 1000, READ ROWS 1000 TO default\n" + assert instance.query("SHOW CREATE QUOTA myQuota2") == "CREATE QUOTA myQuota2 KEYED BY \\'client key or user name\\' FOR RANDOMIZED INTERVAL 1 HOUR MAX RESULT ROWS 4000, RESULT BYTES 400000, READ ROWS 4000, READ BYTES 400000, EXECUTION TIME 60, FOR INTERVAL 1 MONTH MAX EXECUTION TIME 1800\n" expected_usage = "myQuota key=\\\\'default\\\\' interval=\[.*\] queries=1/1000 errors=0 result_rows=50 result_bytes=200 read_rows=50/1000 read_bytes=200 execution_time=.*" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) @@ -212,9 +212,9 @@ def test_dcl_management(): assert instance.query("SHOW QUOTAS") == "" assert instance.query("SHOW QUOTA USAGE") == "" - instance.query("CREATE QUOTA qA FOR INTERVAL 15 MONTH SET MAX QUERIES = 123 TO CURRENT_USER") + instance.query("CREATE QUOTA qA FOR INTERVAL 15 MONTH MAX QUERIES 123 TO CURRENT_USER") assert instance.query("SHOW QUOTAS") == "qA\n" - assert instance.query("SHOW CREATE QUOTA qA") == "CREATE QUOTA qA KEYED BY \\'none\\' FOR INTERVAL 5 QUARTER MAX QUERIES = 123 TO default\n" + assert instance.query("SHOW CREATE QUOTA qA") == "CREATE QUOTA qA KEYED BY \\'none\\' FOR INTERVAL 5 QUARTER MAX QUERIES 123 TO default\n" expected_usage = "qA key=\\\\'\\\\' interval=\[.*\] queries=0/123 errors=0 result_rows=0 result_bytes=0 read_rows=0 read_bytes=0 execution_time=.*" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) @@ -222,14 +222,14 @@ def test_dcl_management(): expected_usage = "qA key=\\\\'\\\\' interval=\[.*\] queries=1/123 errors=0 result_rows=50 result_bytes=200 read_rows=50 read_bytes=200 execution_time=.*" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) - instance.query("ALTER QUOTA qA FOR INTERVAL 15 MONTH MAX QUERIES = 321, MAX ERRORS = 10, FOR INTERVAL 0.5 HOUR MAX EXECUTION TIME = 0.5") - assert instance.query("SHOW CREATE QUOTA qA") == "CREATE QUOTA qA KEYED BY \\'none\\' FOR INTERVAL 30 MINUTE MAX EXECUTION TIME = 0.5, FOR INTERVAL 5 QUARTER MAX QUERIES = 321, MAX ERRORS = 10 TO default\n" + instance.query("ALTER QUOTA qA FOR INTERVAL 15 MONTH MAX QUERIES 321, MAX ERRORS 10, FOR INTERVAL 0.5 HOUR MAX EXECUTION TIME 0.5") + assert instance.query("SHOW CREATE QUOTA qA") == "CREATE QUOTA qA KEYED BY \\'none\\' FOR INTERVAL 30 MINUTE MAX EXECUTION TIME 0.5, FOR INTERVAL 5 QUARTER MAX QUERIES 321, ERRORS 10 TO default\n" expected_usage = "qA key=\\\\'\\\\' interval=\[.*\] queries=0 errors=0 result_rows=0 result_bytes=0 read_rows=0 read_bytes=0 execution_time=.*/0.5\n"\ "qA key=\\\\'\\\\' interval=\[.*\] queries=1/321 errors=0/10 result_rows=50 result_bytes=200 read_rows=50 read_bytes=200 execution_time=.*" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) - instance.query("ALTER QUOTA qA FOR INTERVAL 15 MONTH UNSET TRACKING, FOR RANDOMIZED INTERVAL 16 MONTH SET TRACKING, FOR INTERVAL 1800 SECOND UNSET TRACKING") - assert instance.query("SHOW CREATE QUOTA qA") == "CREATE QUOTA qA KEYED BY \\'none\\' FOR RANDOMIZED INTERVAL 16 MONTH TRACKING TO default\n" + instance.query("ALTER QUOTA qA FOR INTERVAL 15 MONTH NO LIMITS, FOR RANDOMIZED INTERVAL 16 MONTH TRACKING ONLY, FOR INTERVAL 1800 SECOND NO LIMITS") + assert instance.query("SHOW CREATE QUOTA qA") == "CREATE QUOTA qA KEYED BY \\'none\\' FOR RANDOMIZED INTERVAL 16 MONTH TRACKING ONLY TO default\n" expected_usage = "qA key=\\\\'\\\\' interval=\[.*\] queries=0 errors=0 result_rows=0 result_bytes=0 read_rows=0 read_bytes=0 execution_time=.*" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) @@ -238,7 +238,7 @@ def test_dcl_management(): assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) instance.query("ALTER QUOTA qA RENAME TO qB") - assert instance.query("SHOW CREATE QUOTA qB") == "CREATE QUOTA qB KEYED BY \\'none\\' FOR RANDOMIZED INTERVAL 16 MONTH TRACKING TO default\n" + assert instance.query("SHOW CREATE QUOTA qB") == "CREATE QUOTA qB KEYED BY \\'none\\' FOR RANDOMIZED INTERVAL 16 MONTH TRACKING ONLY TO default\n" expected_usage = "qB key=\\\\'\\\\' interval=\[.*\] queries=1 errors=0 result_rows=50 result_bytes=200 read_rows=50 read_bytes=200 execution_time=.*" assert re.match(expected_usage, instance.query("SHOW QUOTA USAGE")) diff --git a/tests/queries/0_stateless/01033_quota_dcl.reference b/tests/queries/0_stateless/01033_quota_dcl.reference index 7f92f992dd5..7bd2d2923d2 100644 --- a/tests/queries/0_stateless/01033_quota_dcl.reference +++ b/tests/queries/0_stateless/01033_quota_dcl.reference @@ -1,2 +1,2 @@ default -CREATE QUOTA default KEYED BY \'user name\' FOR INTERVAL 1 HOUR TRACKING TO default, readonly +CREATE QUOTA default KEYED BY \'user name\' FOR INTERVAL 1 HOUR TRACKING ONLY TO default, readonly From d992e408d8432ba86289fe712096e1ac484086c3 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Wed, 8 Apr 2020 21:01:42 +0300 Subject: [PATCH 5/9] Disable creating row policies for insert, update, delete because those filters are not supported. --- src/Parsers/ParserCreateRowPolicyQuery.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Parsers/ParserCreateRowPolicyQuery.cpp b/src/Parsers/ParserCreateRowPolicyQuery.cpp index 8bfe54b87b2..75c21cd930a 100644 --- a/src/Parsers/ParserCreateRowPolicyQuery.cpp +++ b/src/Parsers/ParserCreateRowPolicyQuery.cpp @@ -83,14 +83,13 @@ namespace static constexpr char delete_op[] = "DELETE"; std::vector ops; - bool keyword_for = false; if (ParserKeyword{"FOR"}.ignore(pos, expected)) { - keyword_for = true; do { if (ParserKeyword{"SELECT"}.ignore(pos, expected)) ops.push_back(select_op); +#if 0 /// INSERT, UPDATE, DELETE are not supported yet else if (ParserKeyword{"INSERT"}.ignore(pos, expected)) ops.push_back(insert_op); else if (ParserKeyword{"UPDATE"}.ignore(pos, expected)) @@ -100,6 +99,7 @@ namespace else if (ParserKeyword{"ALL"}.ignore(pos, expected)) { } +#endif else return false; } @@ -109,9 +109,11 @@ namespace if (ops.empty()) { ops.push_back(select_op); +#if 0 /// INSERT, UPDATE, DELETE are not supported yet ops.push_back(insert_op); ops.push_back(update_op); ops.push_back(delete_op); +#endif } std::optional filter; @@ -123,14 +125,15 @@ namespace if (!parseConditionalExpression(pos, expected, filter)) return false; } +#if 0 /// INSERT, UPDATE, DELETE are not supported yet if (ParserKeyword{"WITH CHECK"}.ignore(pos, expected)) { keyword_with_check = true; if (!parseConditionalExpression(pos, expected, check)) return false; } - - if (!keyword_for && !keyword_using && !keyword_with_check) +#endif + if (!keyword_using && !keyword_with_check) return false; if (filter && !check && !alter) From 4d93577791414f62b586895644cacacd8e861ad8 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Thu, 9 Apr 2020 00:10:00 +0300 Subject: [PATCH 6/9] PREWHERE can be used now by user without row filtering. --- src/Access/EnabledRowPolicies.cpp | 28 +++--- src/Access/RowPolicyCache.cpp | 90 ++----------------- src/Parsers/makeASTForLogicalFunction.cpp | 103 ++++++++++++++++++++++ src/Parsers/makeASTForLogicalFunction.h | 19 ++++ tests/integration/test_row_policy/test.py | 3 + 5 files changed, 146 insertions(+), 97 deletions(-) create mode 100644 src/Parsers/makeASTForLogicalFunction.cpp create mode 100644 src/Parsers/makeASTForLogicalFunction.h diff --git a/src/Access/EnabledRowPolicies.cpp b/src/Access/EnabledRowPolicies.cpp index a525fb65606..56c73aaf40d 100644 --- a/src/Access/EnabledRowPolicies.cpp +++ b/src/Access/EnabledRowPolicies.cpp @@ -1,7 +1,5 @@ #include -#include -#include -#include +#include #include #include @@ -35,19 +33,17 @@ ASTPtr EnabledRowPolicies::getCondition(const String & database, const String & ASTPtr EnabledRowPolicies::getCondition(const String & database, const String & table_name, ConditionType type, const ASTPtr & extra_condition) const { - ASTPtr main_condition = getCondition(database, table_name, type); - if (!main_condition) - return extra_condition; - if (!extra_condition) - return main_condition; - auto function = std::make_shared(); - auto exp_list = std::make_shared(); - function->name = "and"; - function->arguments = exp_list; - function->children.push_back(exp_list); - exp_list->children.push_back(main_condition); - exp_list->children.push_back(extra_condition); - return function; + ASTPtr condition = getCondition(database, table_name, type); + if (condition && extra_condition) + condition = makeASTForLogicalAnd({condition, extra_condition}); + else if (!condition) + condition = extra_condition; + + bool value; + if (tryGetLiteralBool(condition.get(), value) && value) + condition = nullptr; /// The condition is always true, no need to check it. + + return condition; } diff --git a/src/Access/RowPolicyCache.cpp b/src/Access/RowPolicyCache.cpp index 9509923adbf..44f2cd160d4 100644 --- a/src/Access/RowPolicyCache.cpp +++ b/src/Access/RowPolicyCache.cpp @@ -1,97 +1,19 @@ #include #include #include -#include -#include #include #include +#include #include #include #include #include -#include -#include namespace DB { namespace { - bool tryGetLiteralBool(const IAST & ast, bool & value) - { - try - { - if (const ASTLiteral * literal = ast.as()) - { - value = !literal->value.isNull() && applyVisitor(FieldVisitorConvertToNumber(), literal->value); - return true; - } - return false; - } - catch (...) - { - return false; - } - } - - ASTPtr applyFunctionAND(ASTs arguments) - { - bool const_arguments = true; - boost::range::remove_erase_if(arguments, [&](const ASTPtr & argument) -> bool - { - bool b; - if (!tryGetLiteralBool(*argument, b)) - return false; - const_arguments &= b; - return true; - }); - - if (!const_arguments) - return std::make_shared(Field{UInt8(0)}); - if (arguments.empty()) - return std::make_shared(Field{UInt8(1)}); - if (arguments.size() == 1) - return arguments[0]; - - auto function = std::make_shared(); - auto exp_list = std::make_shared(); - function->name = "and"; - function->arguments = exp_list; - function->children.push_back(exp_list); - exp_list->children = std::move(arguments); - return function; - } - - - ASTPtr applyFunctionOR(ASTs arguments) - { - bool const_arguments = false; - boost::range::remove_erase_if(arguments, [&](const ASTPtr & argument) -> bool - { - bool b; - if (!tryGetLiteralBool(*argument, b)) - return false; - const_arguments |= b; - return true; - }); - - if (const_arguments) - return std::make_shared(Field{UInt8(1)}); - if (arguments.empty()) - return std::make_shared(Field{UInt8(0)}); - if (arguments.size() == 1) - return arguments[0]; - - auto function = std::make_shared(); - auto exp_list = std::make_shared(); - function->name = "or"; - function->arguments = exp_list; - function->children.push_back(exp_list); - exp_list->children = std::move(arguments); - return function; - } - - using ConditionType = RowPolicy::ConditionType; constexpr size_t MAX_CONDITION_TYPE = RowPolicy::MAX_CONDITION_TYPE; @@ -111,10 +33,16 @@ namespace ASTPtr getResult() && { /// Process permissive conditions. - restrictions.push_back(applyFunctionOR(std::move(permissions))); + restrictions.push_back(makeASTForLogicalOr(std::move(permissions))); /// Process restrictive conditions. - return applyFunctionAND(std::move(restrictions)); + auto condition = makeASTForLogicalAnd(std::move(restrictions)); + + bool value; + if (tryGetLiteralBool(condition.get(), value) && value) + condition = nullptr; /// The condition is always true, no need to check it. + + return condition; } private: diff --git a/src/Parsers/makeASTForLogicalFunction.cpp b/src/Parsers/makeASTForLogicalFunction.cpp new file mode 100644 index 00000000000..eaae38740aa --- /dev/null +++ b/src/Parsers/makeASTForLogicalFunction.cpp @@ -0,0 +1,103 @@ +#include +#include +#include +#include +#include + + +namespace DB +{ +ASTPtr makeASTForLogicalNot(ASTPtr argument) +{ + bool b; + if (tryGetLiteralBool(argument.get(), b)) + return std::make_shared(Field{UInt8(!b)}); + + auto function = std::make_shared(); + auto exp_list = std::make_shared(); + function->name = "not"; + function->arguments = exp_list; + function->children.push_back(exp_list); + exp_list->children.push_back(argument); + return function; +} + + +ASTPtr makeASTForLogicalAnd(ASTs && arguments) +{ + bool partial_result = true; + boost::range::remove_erase_if(arguments, [&](const ASTPtr & argument) -> bool + { + bool b; + if (!tryGetLiteralBool(argument.get(), b)) + return false; + partial_result &= b; + return true; + }); + + if (!partial_result) + return std::make_shared(Field{UInt8(0)}); + if (arguments.empty()) + return std::make_shared(Field{UInt8(1)}); + if (arguments.size() == 1) + return arguments[0]; + + auto function = std::make_shared(); + auto exp_list = std::make_shared(); + function->name = "and"; + function->arguments = exp_list; + function->children.push_back(exp_list); + exp_list->children = std::move(arguments); + return function; +} + + +ASTPtr makeASTForLogicalOr(ASTs && arguments) +{ + bool partial_result = false; + boost::range::remove_erase_if(arguments, [&](const ASTPtr & argument) -> bool + { + bool b; + if (!tryGetLiteralBool(argument.get(), b)) + return false; + partial_result |= b; + return true; + }); + + if (partial_result) + return std::make_shared(Field{UInt8(1)}); + if (arguments.empty()) + return std::make_shared(Field{UInt8(0)}); + if (arguments.size() == 1) + return arguments[0]; + + auto function = std::make_shared(); + auto exp_list = std::make_shared(); + function->name = "or"; + function->arguments = exp_list; + function->children.push_back(exp_list); + exp_list->children = std::move(arguments); + return function; +} + + +bool tryGetLiteralBool(const IAST * ast, bool & value) +{ + if (!ast) + return false; + + try + { + if (const ASTLiteral * literal = ast->as()) + { + value = !literal->value.isNull() && applyVisitor(FieldVisitorConvertToNumber(), literal->value); + return true; + } + return false; + } + catch (...) + { + return false; + } +} +} diff --git a/src/Parsers/makeASTForLogicalFunction.h b/src/Parsers/makeASTForLogicalFunction.h new file mode 100644 index 00000000000..5c1096cab6e --- /dev/null +++ b/src/Parsers/makeASTForLogicalFunction.h @@ -0,0 +1,19 @@ +#pragma once + +#include + + +namespace DB +{ +/// Makes an AST calculating NOT argument. +ASTPtr makeASTForLogicalNot(ASTPtr argument); + +/// Makes an AST calculating argument1 AND argument2 AND ... AND argumentN. +ASTPtr makeASTForLogicalAnd(ASTs && arguments); + +/// Makes an AST calculating argument1 OR argument2 OR ... OR argumentN. +ASTPtr makeASTForLogicalOr(ASTs && arguments); + +/// Tries to extract a literal bool from AST. +bool tryGetLiteralBool(const IAST * ast, bool & value); +} diff --git a/tests/integration/test_row_policy/test.py b/tests/integration/test_row_policy/test.py index 7087e6aafae..3a5b7340528 100644 --- a/tests/integration/test_row_policy/test.py +++ b/tests/integration/test_row_policy/test.py @@ -113,6 +113,9 @@ def test_prewhere_not_supported(): assert expected_error in instance.query_and_get_error("SELECT * FROM mydb.filtered_table2 PREWHERE 1") assert expected_error in instance.query_and_get_error("SELECT * FROM mydb.filtered_table3 PREWHERE 1") + # However PREWHERE should still work for user without filtering. + assert instance.query("SELECT * FROM mydb.filtered_table1 PREWHERE 1", user="another") == "0\t0\n0\t1\n1\t0\n1\t1\n" + def test_single_table_name(): copy_policy_xml('tag_with_table_name.xml') From f0d3547b8f19ac0747849e28cfeb6b14b1f67896 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Thu, 9 Apr 2020 02:01:41 +0300 Subject: [PATCH 7/9] Parser can parse "ON CLUSTER" in other places of SQL commands too. --- src/Parsers/ParserCreateQuotaQuery.cpp | 22 +++++++++++------ src/Parsers/ParserCreateRoleQuery.cpp | 20 ++++++++++------ src/Parsers/ParserCreateRowPolicyQuery.cpp | 22 +++++++++++------ .../ParserCreateSettingsProfileQuery.cpp | 23 ++++++++++++------ src/Parsers/ParserCreateUserQuery.cpp | 19 +++++++++------ src/Parsers/ParserGrantQuery.cpp | 24 +++++++++++++++---- 6 files changed, 90 insertions(+), 40 deletions(-) diff --git a/src/Parsers/ParserCreateQuotaQuery.cpp b/src/Parsers/ParserCreateQuotaQuery.cpp index 8bbd2127922..6007d6206ec 100644 --- a/src/Parsers/ParserCreateQuotaQuery.cpp +++ b/src/Parsers/ParserCreateQuotaQuery.cpp @@ -190,6 +190,14 @@ namespace return true; }); } + + bool parseOnCluster(IParserBase::Pos & pos, Expected & expected, String & cluster) + { + return IParserBase::wrapParseImpl(pos, [&] + { + return ParserKeyword{"ON"}.ignore(pos, expected) && ASTQueryWithOnCluster::parse(pos, cluster, expected); + }); + } } @@ -229,16 +237,10 @@ bool ParserCreateQuotaQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe if (!parseIdentifierOrStringLiteral(pos, expected, name)) return false; - String cluster; - if (ParserKeyword{"ON"}.ignore(pos, expected)) - { - if (!ASTQueryWithOnCluster::parse(pos, cluster, expected)) - return false; - } - String new_name; std::optional key_type; std::vector all_limits; + String cluster; while (true) { @@ -251,12 +253,18 @@ bool ParserCreateQuotaQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe if (parseAllLimits(pos, expected, all_limits)) continue; + if (cluster.empty() && parseOnCluster(pos, expected, cluster)) + continue; + break; } std::shared_ptr roles; parseToRoles(pos, expected, attach_mode, roles); + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); + auto query = std::make_shared(); node = query; diff --git a/src/Parsers/ParserCreateRoleQuery.cpp b/src/Parsers/ParserCreateRoleQuery.cpp index 05143108480..2a6f2dd2c90 100644 --- a/src/Parsers/ParserCreateRoleQuery.cpp +++ b/src/Parsers/ParserCreateRoleQuery.cpp @@ -41,6 +41,14 @@ namespace return true; }); } + + bool parseOnCluster(IParserBase::Pos & pos, Expected & expected, String & cluster) + { + return IParserBase::wrapParseImpl(pos, [&] + { + return ParserKeyword{"ON"}.ignore(pos, expected) && ASTQueryWithOnCluster::parse(pos, cluster, expected); + }); + } } @@ -80,15 +88,10 @@ bool ParserCreateRoleQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec if (!parseRoleName(pos, expected, name)) return false; - String cluster; - if (ParserKeyword{"ON"}.ignore(pos, expected)) - { - if (!ASTQueryWithOnCluster::parse(pos, cluster, expected)) - return false; - } - String new_name; std::shared_ptr settings; + String cluster; + while (true) { if (alter && parseRenameTo(pos, expected, new_name)) @@ -97,6 +100,9 @@ bool ParserCreateRoleQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec if (parseSettings(pos, expected, attach_mode, settings)) continue; + if (cluster.empty() && parseOnCluster(pos, expected, cluster)) + continue; + break; } diff --git a/src/Parsers/ParserCreateRowPolicyQuery.cpp b/src/Parsers/ParserCreateRowPolicyQuery.cpp index 75c21cd930a..b6840f0ed6a 100644 --- a/src/Parsers/ParserCreateRowPolicyQuery.cpp +++ b/src/Parsers/ParserCreateRowPolicyQuery.cpp @@ -203,6 +203,14 @@ namespace return true; }); } + + bool parseOnCluster(IParserBase::Pos & pos, Expected & expected, String & cluster) + { + return IParserBase::wrapParseImpl(pos, [&] + { + return ParserKeyword{"ON"}.ignore(pos, expected) && ASTQueryWithOnCluster::parse(pos, cluster, expected); + }); + } } @@ -246,16 +254,10 @@ bool ParserCreateRowPolicyQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & || !parseDatabaseAndTableName(pos, expected, database, table_name)) return false; - String cluster; - if (ParserKeyword{"ON"}.ignore(pos, expected)) - { - if (!ASTQueryWithOnCluster::parse(pos, cluster, expected)) - return false; - } - String new_policy_name; std::optional is_restrictive; std::vector> conditions; + String cluster; while (true) { @@ -268,12 +270,18 @@ bool ParserCreateRowPolicyQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & if (parseMultipleConditions(pos, expected, alter, conditions)) continue; + if (cluster.empty() && parseOnCluster(pos, expected, cluster)) + continue; + break; } std::shared_ptr roles; parseToRoles(pos, expected, attach_mode, roles); + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); + auto query = std::make_shared(); node = query; diff --git a/src/Parsers/ParserCreateSettingsProfileQuery.cpp b/src/Parsers/ParserCreateSettingsProfileQuery.cpp index 5b33fed2fa0..83d0f0c1d91 100644 --- a/src/Parsers/ParserCreateSettingsProfileQuery.cpp +++ b/src/Parsers/ParserCreateSettingsProfileQuery.cpp @@ -57,6 +57,14 @@ namespace return true; }); } + + bool parseOnCluster(IParserBase::Pos & pos, Expected & expected, String & cluster) + { + return IParserBase::wrapParseImpl(pos, [&] + { + return ParserKeyword{"ON"}.ignore(pos, expected) && ASTQueryWithOnCluster::parse(pos, cluster, expected); + }); + } } @@ -96,15 +104,10 @@ bool ParserCreateSettingsProfileQuery::parseImpl(Pos & pos, ASTPtr & node, Expec if (!parseIdentifierOrStringLiteral(pos, expected, name)) return false; - String cluster; - if (ParserKeyword{"ON"}.ignore(pos, expected)) - { - if (!ASTQueryWithOnCluster::parse(pos, cluster, expected)) - return false; - } - String new_name; std::shared_ptr settings; + String cluster; + while (true) { if (alter && parseRenameTo(pos, expected, new_name)) @@ -113,12 +116,18 @@ bool ParserCreateSettingsProfileQuery::parseImpl(Pos & pos, ASTPtr & node, Expec if (parseSettings(pos, expected, attach_mode, settings)) continue; + if (cluster.empty() && parseOnCluster(pos, expected, cluster)) + continue; + break; } std::shared_ptr to_roles; parseToRoles(pos, expected, attach_mode, to_roles); + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); + auto query = std::make_shared(); node = query; diff --git a/src/Parsers/ParserCreateUserQuery.cpp b/src/Parsers/ParserCreateUserQuery.cpp index 3968c26d42e..28483cc76ec 100644 --- a/src/Parsers/ParserCreateUserQuery.cpp +++ b/src/Parsers/ParserCreateUserQuery.cpp @@ -250,6 +250,14 @@ namespace return true; }); } + + bool parseOnCluster(IParserBase::Pos & pos, Expected & expected, String & cluster) + { + return IParserBase::wrapParseImpl(pos, [&] + { + return ParserKeyword{"ON"}.ignore(pos, expected) && ASTQueryWithOnCluster::parse(pos, cluster, expected); + }); + } } @@ -290,13 +298,6 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec if (!parseUserName(pos, expected, name, host_pattern)) return false; - String cluster; - if (ParserKeyword{"ON"}.ignore(pos, expected)) - { - if (!ASTQueryWithOnCluster::parse(pos, cluster, expected)) - return false; - } - String new_name; std::optional new_host_pattern; std::optional authentication; @@ -305,6 +306,7 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec std::optional remove_hosts; std::shared_ptr default_roles; std::shared_ptr settings; + String cluster; while (true) { @@ -320,6 +322,9 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec if (!default_roles && parseDefaultRoles(pos, expected, attach_mode, default_roles)) continue; + if (cluster.empty() && parseOnCluster(pos, expected, cluster)) + continue; + if (alter) { if (new_name.empty() && parseRenameTo(pos, expected, new_name, new_host_pattern)) diff --git a/src/Parsers/ParserGrantQuery.cpp b/src/Parsers/ParserGrantQuery.cpp index f6eecbe5dba..64dde8f6524 100644 --- a/src/Parsers/ParserGrantQuery.cpp +++ b/src/Parsers/ParserGrantQuery.cpp @@ -237,6 +237,14 @@ namespace return true; }); } + + bool parseOnCluster(IParserBase::Pos & pos, Expected & expected, String & cluster) + { + return IParserBase::wrapParseImpl(pos, [&] + { + return ParserKeyword{"ON"}.ignore(pos, expected) && ASTQueryWithOnCluster::parse(pos, cluster, expected); + }); + } } @@ -260,11 +268,8 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) return false; String cluster; - if (ParserKeyword{"ON"}.ignore(pos, expected)) - { - if (!ASTQueryWithOnCluster::parse(pos, cluster, expected)) - return false; - } + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); bool grant_option = false; bool admin_option = false; @@ -281,10 +286,16 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) if (!parseAccessRightsElements(pos, expected, elements) && !parseRoles(pos, expected, attach, roles)) return false; + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); + std::shared_ptr to_roles; if (!parseToRoles(pos, expected, kind, to_roles)) return false; + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); + if (kind == Kind::GRANT) { if (ParserKeyword{"WITH GRANT OPTION"}.ignore(pos, expected)) @@ -293,6 +304,9 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) admin_option = true; } + if (cluster.empty()) + parseOnCluster(pos, expected, cluster); + if (grant_option && roles) throw Exception("GRANT OPTION should be specified for access types", ErrorCodes::SYNTAX_ERROR); if (admin_option && !elements.empty()) From ed2562b3f468ecab6b0bf49ea95ec9487a6524f2 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Thu, 9 Apr 2020 02:53:41 +0300 Subject: [PATCH 8/9] Add new words to client's suggest. --- programs/client/Suggest.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/programs/client/Suggest.cpp b/programs/client/Suggest.cpp index f7141449f54..8fffbec4fab 100644 --- a/programs/client/Suggest.cpp +++ b/programs/client/Suggest.cpp @@ -67,16 +67,19 @@ void Suggest::load(const ConnectionParameters & connection_parameters, size_t su Suggest::Suggest() { /// Keywords may be not up to date with ClickHouse parser. - words = {"CREATE", "DATABASE", "IF", "NOT", "EXISTS", "TEMPORARY", "TABLE", "ON", "CLUSTER", "DEFAULT", - "MATERIALIZED", "ALIAS", "ENGINE", "AS", "VIEW", "POPULATE", "SETTINGS", "ATTACH", "DETACH", "DROP", - "RENAME", "TO", "ALTER", "ADD", "MODIFY", "CLEAR", "COLUMN", "AFTER", "COPY", "PROJECT", - "PRIMARY", "KEY", "CHECK", "PARTITION", "PART", "FREEZE", "FETCH", "FROM", "SHOW", "INTO", - "OUTFILE", "FORMAT", "TABLES", "DATABASES", "LIKE", "PROCESSLIST", "CASE", "WHEN", "THEN", "ELSE", - "END", "DESCRIBE", "DESC", "USE", "SET", "OPTIMIZE", "FINAL", "DEDUPLICATE", "INSERT", "VALUES", - "SELECT", "DISTINCT", "SAMPLE", "ARRAY", "JOIN", "GLOBAL", "LOCAL", "ANY", "ALL", "INNER", - "LEFT", "RIGHT", "FULL", "OUTER", "CROSS", "USING", "PREWHERE", "WHERE", "GROUP", "BY", - "WITH", "TOTALS", "HAVING", "ORDER", "COLLATE", "LIMIT", "UNION", "AND", "OR", "ASC", - "IN", "KILL", "QUERY", "SYNC", "ASYNC", "TEST", "BETWEEN", "TRUNCATE"}; + words = {"CREATE", "DATABASE", "IF", "NOT", "EXISTS", "TEMPORARY", "TABLE", "ON", "CLUSTER", "DEFAULT", + "MATERIALIZED", "ALIAS", "ENGINE", "AS", "VIEW", "POPULATE", "SETTINGS", "ATTACH", "DETACH", "DROP", + "RENAME", "TO", "ALTER", "ADD", "MODIFY", "CLEAR", "COLUMN", "AFTER", "COPY", "PROJECT", + "PRIMARY", "KEY", "CHECK", "PARTITION", "PART", "FREEZE", "FETCH", "FROM", "SHOW", "INTO", + "OUTFILE", "FORMAT", "TABLES", "DATABASES", "LIKE", "PROCESSLIST", "CASE", "WHEN", "THEN", "ELSE", + "END", "DESCRIBE", "DESC", "USE", "SET", "OPTIMIZE", "FINAL", "DEDUPLICATE", "INSERT", "VALUES", + "SELECT", "DISTINCT", "SAMPLE", "ARRAY", "JOIN", "GLOBAL", "LOCAL", "ANY", "ALL", "INNER", + "LEFT", "RIGHT", "FULL", "OUTER", "CROSS", "USING", "PREWHERE", "WHERE", "GROUP", "BY", + "WITH", "TOTALS", "HAVING", "ORDER", "COLLATE", "LIMIT", "UNION", "AND", "OR", "ASC", + "IN", "KILL", "QUERY", "SYNC", "ASYNC", "TEST", "BETWEEN", "TRUNCATE", "USER", "ROLE", + "PROFILE", "QUOTA", "POLICY", "ROW", "GRANT", "REVOKE", "OPTION", "ADMIN", "EXCEPT", "REPLACE", + "IDENTIFIED", "HOST", "NAME", "READONLY", "WRITABLE", "PERMISSIVE", "FOR", "RESTRICTIVE", "FOR", "RANDOMIZED", + "INTERVAL", "LIMITS", "ONLY", "TRACKING", "IP", "REGEXP"}; } void Suggest::loadImpl(Connection & connection, const ConnectionTimeouts & timeouts, size_t suggestion_limit) From 12336a9ece3d9b6d2073c6d6168a976bfec65b88 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Thu, 9 Apr 2020 02:57:45 +0300 Subject: [PATCH 9/9] Use "CREATE USER HOST REGEXP" instead of "CREATE USER HOST NAME REGEXP". --- src/Parsers/ASTCreateUserQuery.cpp | 2 +- src/Parsers/ASTCreateUserQuery.h | 4 ++-- src/Parsers/ParserCreateUserQuery.cpp | 2 +- src/Parsers/ParserCreateUserQuery.h | 4 ++-- .../0_stateless/01075_allowed_client_hosts.reference | 8 ++++---- tests/queries/0_stateless/01075_allowed_client_hosts.sql | 8 ++++---- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Parsers/ASTCreateUserQuery.cpp b/src/Parsers/ASTCreateUserQuery.cpp index d901ed8f5a1..c8e2a76dfa2 100644 --- a/src/Parsers/ASTCreateUserQuery.cpp +++ b/src/Parsers/ASTCreateUserQuery.cpp @@ -109,7 +109,7 @@ namespace { if (std::exchange(need_comma, true)) settings.ostr << ", "; - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "NAME REGEXP " << (settings.hilite ? IAST::hilite_none : ""); + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "REGEXP " << (settings.hilite ? IAST::hilite_none : ""); bool need_comma2 = false; for (const auto & host_regexp : name_regexps) { diff --git a/src/Parsers/ASTCreateUserQuery.h b/src/Parsers/ASTCreateUserQuery.h index 5a5cc0d9550..54dc51d783b 100644 --- a/src/Parsers/ASTCreateUserQuery.h +++ b/src/Parsers/ASTCreateUserQuery.h @@ -13,14 +13,14 @@ class ASTSettingsProfileElements; /** CREATE USER [IF NOT EXISTS | OR REPLACE] name * [IDENTIFIED [WITH {NO_PASSWORD|PLAINTEXT_PASSWORD|SHA256_PASSWORD|SHA256_HASH|DOUBLE_SHA1_PASSWORD|DOUBLE_SHA1_HASH}] BY {'password'|'hash'}] - * [HOST {LOCAL | NAME 'name' | NAME REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] + * [HOST {LOCAL | NAME 'name' | REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] * [DEFAULT ROLE role [,...]] * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] * * ALTER USER [IF EXISTS] name * [RENAME TO new_name] * [IDENTIFIED [WITH {PLAINTEXT_PASSWORD|SHA256_PASSWORD|DOUBLE_SHA1_PASSWORD}] BY {'password'|'hash'}] - * [[ADD|DROP] HOST {LOCAL | NAME 'name' | NAME REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] + * [[ADD|DROP] HOST {LOCAL | NAME 'name' | REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] * [DEFAULT ROLE role [,...] | ALL | ALL EXCEPT role [,...] ] * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] */ diff --git a/src/Parsers/ParserCreateUserQuery.cpp b/src/Parsers/ParserCreateUserQuery.cpp index 28483cc76ec..76a06a0282f 100644 --- a/src/Parsers/ParserCreateUserQuery.cpp +++ b/src/Parsers/ParserCreateUserQuery.cpp @@ -166,7 +166,7 @@ namespace { new_hosts.addLocalHost(); } - else if (ParserKeyword{"NAME REGEXP"}.ignore(pos, expected)) + else if (ParserKeyword{"REGEXP"}.ignore(pos, expected)) { ASTPtr ast; if (!ParserList{std::make_unique(), std::make_unique(TokenType::Comma), false}.parse(pos, ast, expected)) diff --git a/src/Parsers/ParserCreateUserQuery.h b/src/Parsers/ParserCreateUserQuery.h index 4b2af34c003..d609894a7ec 100644 --- a/src/Parsers/ParserCreateUserQuery.h +++ b/src/Parsers/ParserCreateUserQuery.h @@ -8,13 +8,13 @@ namespace DB /** Parses queries like * CREATE USER [IF NOT EXISTS | OR REPLACE] name * [IDENTIFIED [WITH {NO_PASSWORD|PLAINTEXT_PASSWORD|SHA256_PASSWORD|SHA256_HASH|DOUBLE_SHA1_PASSWORD|DOUBLE_SHA1_HASH}] BY {'password'|'hash'}] - * [HOST {LOCAL | NAME 'name' | NAME REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] + * [HOST {LOCAL | NAME 'name' | REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] * * ALTER USER [IF EXISTS] name * [RENAME TO new_name] * [IDENTIFIED [WITH {PLAINTEXT_PASSWORD|SHA256_PASSWORD|DOUBLE_SHA1_PASSWORD}] BY {'password'|'hash'}] - * [[ADD|DROP] HOST {LOCAL | NAME 'name' | NAME REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] + * [[ADD|DROP] HOST {LOCAL | NAME 'name' | REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] * [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE] | PROFILE 'profile_name'] [,...] */ class ParserCreateUserQuery : public IParserBase diff --git a/tests/queries/0_stateless/01075_allowed_client_hosts.reference b/tests/queries/0_stateless/01075_allowed_client_hosts.reference index 0082653059c..73f54c6027a 100644 --- a/tests/queries/0_stateless/01075_allowed_client_hosts.reference +++ b/tests/queries/0_stateless/01075_allowed_client_hosts.reference @@ -8,10 +8,10 @@ CREATE USER test_user_01075 HOST LOCAL, IP \'2001:db8:11a3:9d7:1f34:8a2e:7a0:765 CREATE USER test_user_01075 HOST LOCAL CREATE USER test_user_01075 HOST NONE CREATE USER test_user_01075 HOST LIKE \'@.somesite.com\' -CREATE USER test_user_01075 HOST NAME REGEXP \'.*.anothersite.com\' -CREATE USER test_user_01075 HOST NAME REGEXP \'.*.anothersite.com\', \'.*.anothersite.org\' -CREATE USER test_user_01075 HOST NAME REGEXP \'.*.anothersite2.com\', \'.*.anothersite2.org\' -CREATE USER test_user_01075 HOST NAME REGEXP \'.*.anothersite3.com\', \'.*.anothersite3.org\' +CREATE USER test_user_01075 HOST REGEXP \'.*.anothersite.com\' +CREATE USER test_user_01075 HOST REGEXP \'.*.anothersite.com\', \'.*.anothersite.org\' +CREATE USER test_user_01075 HOST REGEXP \'.*.anothersite2.com\', \'.*.anothersite2.org\' +CREATE USER test_user_01075 HOST REGEXP \'.*.anothersite3.com\', \'.*.anothersite3.org\' CREATE USER `test_user_01075_x@localhost` HOST LOCAL CREATE USER test_user_01075_x CREATE USER `test_user_01075_x@192.168.23.15` HOST LIKE \'192.168.23.15\' diff --git a/tests/queries/0_stateless/01075_allowed_client_hosts.sql b/tests/queries/0_stateless/01075_allowed_client_hosts.sql index e0b1c0f9905..2960a93f0f2 100644 --- a/tests/queries/0_stateless/01075_allowed_client_hosts.sql +++ b/tests/queries/0_stateless/01075_allowed_client_hosts.sql @@ -30,16 +30,16 @@ SHOW CREATE USER test_user_01075; ALTER USER test_user_01075 HOST LIKE '@.somesite.com'; SHOW CREATE USER test_user_01075; -ALTER USER test_user_01075 HOST NAME REGEXP '.*\.anothersite\.com'; +ALTER USER test_user_01075 HOST REGEXP '.*\.anothersite\.com'; SHOW CREATE USER test_user_01075; -ALTER USER test_user_01075 HOST NAME REGEXP '.*\.anothersite\.com', '.*\.anothersite\.org'; +ALTER USER test_user_01075 HOST REGEXP '.*\.anothersite\.com', '.*\.anothersite\.org'; SHOW CREATE USER test_user_01075; -ALTER USER test_user_01075 HOST NAME REGEXP '.*\.anothersite2\.com', NAME REGEXP '.*\.anothersite2\.org'; +ALTER USER test_user_01075 HOST REGEXP '.*\.anothersite2\.com', REGEXP '.*\.anothersite2\.org'; SHOW CREATE USER test_user_01075; -ALTER USER test_user_01075 HOST NAME REGEXP '.*\.anothersite3\.com' HOST NAME REGEXP '.*\.anothersite3\.org'; +ALTER USER test_user_01075 HOST REGEXP '.*\.anothersite3\.com' HOST REGEXP '.*\.anothersite3\.org'; SHOW CREATE USER test_user_01075; DROP USER test_user_01075;