From 9ed708b9027d47b7d0bb8326cdf54dce36afebd7 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Tue, 7 Apr 2020 13:14:49 +0800 Subject: [PATCH 1/3] ISSUES-10056 add some check and support identifier argument for MySQL Database Engine --- src/Databases/DatabaseFactory.cpp | 34 ++++++++++++++++--- .../test_mysql_database_engine/test.py | 7 +++- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 40e5682565d..b6300ab3482 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +50,28 @@ DatabasePtr DatabaseFactory::get( } } +template +static inline ValueType getLiteralValue(const ASTPtr & ast, const String & engine_name) +{ + if (!ast || !ast->as()) + throw Exception("Database engine " + engine_name + " requested literal argument.", ErrorCodes::BAD_ARGUMENTS); + + return ast->as()->value.safeGet(); +} + +[[maybe_unused]] static inline String getIdentifierOrStringLiteral(const ASTPtr & ast, const String & engine_name) +{ + if (ast) + { + if (const auto & literal = ast->as()) + return literal->value.safeGet(); + else if (const auto & identifier = ast->as()) + return identifier->name; + } + + throw Exception("Database engine " + engine_name + " requested literal or identifier argument.", ErrorCodes::BAD_ARGUMENTS); +} + DatabasePtr DatabaseFactory::getImpl( const String & database_name, const String & metadata_path, const ASTStorage * engine_define, Context & context) { @@ -79,11 +102,12 @@ DatabasePtr DatabaseFactory::getImpl( throw Exception("MySQL Database require mysql_hostname, mysql_database_name, mysql_username, mysql_password arguments.", ErrorCodes::BAD_ARGUMENTS); + const auto & arguments = engine->arguments->children; - const auto & host_name_and_port = arguments[0]->as()->value.safeGet(); - const auto & database_name_in_mysql = arguments[1]->as()->value.safeGet(); - const auto & mysql_user_name = arguments[2]->as()->value.safeGet(); - const auto & mysql_user_password = arguments[3]->as()->value.safeGet(); + const auto & host_name_and_port = getLiteralValue(arguments[0], "MySQL"); + const auto & database_name_in_mysql = getIdentifierOrStringLiteral(arguments[1], "MySQL"); + const auto & mysql_user_name = getLiteralValue(arguments[2], "MySQL"); + const auto & mysql_user_password = getLiteralValue(arguments[3], "MySQL"); try { @@ -114,7 +138,7 @@ DatabasePtr DatabaseFactory::getImpl( const auto & arguments = engine->arguments->children; - const auto cache_expiration_time_seconds = arguments[0]->as()->value.safeGet(); + const auto cache_expiration_time_seconds = getLiteralValue(arguments[0], "Lazy"); return std::make_shared(database_name, metadata_path, cache_expiration_time_seconds, context); } diff --git a/tests/integration/test_mysql_database_engine/test.py b/tests/integration/test_mysql_database_engine/test.py index 86e0b9df5fd..42663e46752 100644 --- a/tests/integration/test_mysql_database_engine/test.py +++ b/tests/integration/test_mysql_database_engine/test.py @@ -92,7 +92,7 @@ def test_clickhouse_dml_for_mysql_database(started_cluster): with contextlib.closing(MySQLNodeInstance('root', 'clickhouse', '127.0.0.1', port=3308)) as mysql_node: mysql_node.query("CREATE DATABASE test_database DEFAULT CHARACTER SET 'utf8'") mysql_node.query('CREATE TABLE `test_database`.`test_table` ( `i``d` int(11) NOT NULL, PRIMARY KEY (`i``d`)) ENGINE=InnoDB;') - clickhouse_node.query("CREATE DATABASE test_database ENGINE = MySQL('mysql1:3306', 'test_database', 'root', 'clickhouse')") + clickhouse_node.query("CREATE DATABASE test_database ENGINE = MySQL('mysql1:3306', test_database, 'root', 'clickhouse')") assert clickhouse_node.query("SELECT count() FROM `test_database`.`test_table`").rstrip() == '0' clickhouse_node.query("INSERT INTO `test_database`.`test_table`(`i\`d`) select number from numbers(10000)") @@ -120,3 +120,8 @@ def test_clickhouse_join_for_mysql_database(started_cluster): "LEFT JOIN default.t1_remote_mysql AS s_ref " "ON (s_ref.opco = s.opco AND s_ref.service = s.service)") == '' mysql_node.query("DROP DATABASE test") + +def test_bad_arguments_for_mysql_database_engine(started_cluster): + assert clickhouse_node.query( + "CREATE TABLE default.t1_remote_mysql AS mysql('mysql1:3306', 'test', 't1_mysql_local', root, 'clickhouse')").find( + 'Database engine MySQL requested literal argument.') != -1 From aa0fcf40886f06cd66711071d734198019895348 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Tue, 7 Apr 2020 20:25:01 +0800 Subject: [PATCH 2/3] ISSUES-10056 update docs --- docs/en/engines/database_engines/mysql.md | 4 ++-- docs/ru/database_engines/mysql.md | 4 +--- docs/zh/engines/database_engines/mysql.md | 4 +--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/docs/en/engines/database_engines/mysql.md b/docs/en/engines/database_engines/mysql.md index 678c174e1fb..467a3aa032d 100644 --- a/docs/en/engines/database_engines/mysql.md +++ b/docs/en/engines/database_engines/mysql.md @@ -3,7 +3,7 @@ toc_priority: 30 toc_title: MySQL --- -# Mysql {#mysql} +# MySQL {#mysql} Allows to connect to databases on a remote MySQL server and perform `INSERT` and `SELECT` queries to exchange data between ClickHouse and MySQL. @@ -19,7 +19,7 @@ You cannot perform the following queries: ``` sql CREATE DATABASE [IF NOT EXISTS] db_name [ON CLUSTER cluster] -ENGINE = MySQL('host:port', 'database', 'user', 'password') +ENGINE = MySQL('host:port', ['database' | database], 'user', 'password') ``` **Engine Parameters** diff --git a/docs/ru/database_engines/mysql.md b/docs/ru/database_engines/mysql.md index 420ca370297..45547407be6 100644 --- a/docs/ru/database_engines/mysql.md +++ b/docs/ru/database_engines/mysql.md @@ -6,8 +6,6 @@ Не поддерживаемые виды запросов: -- `ATTACH`/`DETACH` -- `DROP` - `RENAME` - `CREATE TABLE` - `ALTER` @@ -16,7 +14,7 @@ ``` sql CREATE DATABASE [IF NOT EXISTS] db_name [ON CLUSTER cluster] -ENGINE = MySQL('host:port', 'database', 'user', 'password') +ENGINE = MySQL('host:port', ['database' | database], 'user', 'password') ``` **Параметры движка** diff --git a/docs/zh/engines/database_engines/mysql.md b/docs/zh/engines/database_engines/mysql.md index 78844154bce..80ff82ec2d3 100644 --- a/docs/zh/engines/database_engines/mysql.md +++ b/docs/zh/engines/database_engines/mysql.md @@ -7,8 +7,6 @@ MySQL引擎用于将远程的MySQL服务器中的表映射到ClickHouse中，并 但您无法对其执行以下操作： -- `ATTACH`/`DETACH` -- `DROP` - `RENAME` - `CREATE TABLE` - `ALTER` @@ -17,7 +15,7 @@ MySQL引擎用于将远程的MySQL服务器中的表映射到ClickHouse中，并 ``` sql CREATE DATABASE [IF NOT EXISTS] db_name [ON CLUSTER cluster] -ENGINE = MySQL('host:port', 'database', 'user', 'password') +ENGINE = MySQL('host:port', ['database' | database], 'user', 'password') ``` **MySQL数据库引擎参数** From 9eb96b87db4ae5b1fba90640aa65205ad1fc8379 Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Wed, 8 Apr 2020 13:41:11 +0800 Subject: [PATCH 3/3] ISSUES-10056 reused evaluateConstantExpressionOrIdentifierAsLiteral --- src/Databases/DatabaseFactory.cpp | 30 +++++++------------ .../test_mysql_database_engine/test.py | 18 +++++++---- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index b6300ab3482..f1cea04dc29 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -16,6 +16,7 @@ #if USE_MYSQL #include +#include #endif @@ -51,7 +52,7 @@ DatabasePtr DatabaseFactory::get( } template -static inline ValueType getLiteralValue(const ASTPtr & ast, const String & engine_name) +static inline ValueType safeGetLiteralValue(const ASTPtr &ast, const String &engine_name) { if (!ast || !ast->as()) throw Exception("Database engine " + engine_name + " requested literal argument.", ErrorCodes::BAD_ARGUMENTS); @@ -59,19 +60,6 @@ static inline ValueType getLiteralValue(const ASTPtr & ast, const String & engin return ast->as()->value.safeGet(); } -[[maybe_unused]] static inline String getIdentifierOrStringLiteral(const ASTPtr & ast, const String & engine_name) -{ - if (ast) - { - if (const auto & literal = ast->as()) - return literal->value.safeGet(); - else if (const auto & identifier = ast->as()) - return identifier->name; - } - - throw Exception("Database engine " + engine_name + " requested literal or identifier argument.", ErrorCodes::BAD_ARGUMENTS); -} - DatabasePtr DatabaseFactory::getImpl( const String & database_name, const String & metadata_path, const ASTStorage * engine_define, Context & context) { @@ -103,11 +91,13 @@ DatabasePtr DatabaseFactory::getImpl( ErrorCodes::BAD_ARGUMENTS); - const auto & arguments = engine->arguments->children; - const auto & host_name_and_port = getLiteralValue(arguments[0], "MySQL"); - const auto & database_name_in_mysql = getIdentifierOrStringLiteral(arguments[1], "MySQL"); - const auto & mysql_user_name = getLiteralValue(arguments[2], "MySQL"); - const auto & mysql_user_password = getLiteralValue(arguments[3], "MySQL"); + ASTs & arguments = engine->arguments->children; + arguments[1] = evaluateConstantExpressionOrIdentifierAsLiteral(arguments[1], context); + + const auto & host_name_and_port = safeGetLiteralValue(arguments[0], "MySQL"); + const auto & database_name_in_mysql = safeGetLiteralValue(arguments[1], "MySQL"); + const auto & mysql_user_name = safeGetLiteralValue(arguments[2], "MySQL"); + const auto & mysql_user_password = safeGetLiteralValue(arguments[3], "MySQL"); try { @@ -138,7 +128,7 @@ DatabasePtr DatabaseFactory::getImpl( const auto & arguments = engine->arguments->children; - const auto cache_expiration_time_seconds = getLiteralValue(arguments[0], "Lazy"); + const auto cache_expiration_time_seconds = safeGetLiteralValue(arguments[0], "Lazy"); return std::make_shared(database_name, metadata_path, cache_expiration_time_seconds, context); } diff --git a/tests/integration/test_mysql_database_engine/test.py b/tests/integration/test_mysql_database_engine/test.py index 42663e46752..2791cc7b382 100644 --- a/tests/integration/test_mysql_database_engine/test.py +++ b/tests/integration/test_mysql_database_engine/test.py @@ -5,6 +5,7 @@ import pymysql.cursors import pytest from helpers.cluster import ClickHouseCluster +from helpers.client import QueryRuntimeException cluster = ClickHouseCluster(__file__) clickhouse_node = cluster.add_instance('node1', main_configs=['configs/remote_servers.xml'], with_mysql=True) @@ -116,12 +117,17 @@ def test_clickhouse_join_for_mysql_database(started_cluster): clickhouse_node.query("CREATE TABLE default.t1_remote_mysql AS mysql('mysql1:3306','test','t1_mysql_local','root','clickhouse')") clickhouse_node.query("CREATE TABLE default.t2_remote_mysql AS mysql('mysql1:3306','test','t2_mysql_local','root','clickhouse')") assert clickhouse_node.query("SELECT s.pays " - "FROM default.t1_remote_mysql AS s " - "LEFT JOIN default.t1_remote_mysql AS s_ref " - "ON (s_ref.opco = s.opco AND s_ref.service = s.service)") == '' + "FROM default.t1_remote_mysql AS s " + "LEFT JOIN default.t1_remote_mysql AS s_ref " + "ON (s_ref.opco = s.opco AND s_ref.service = s.service)") == '' mysql_node.query("DROP DATABASE test") + def test_bad_arguments_for_mysql_database_engine(started_cluster): - assert clickhouse_node.query( - "CREATE TABLE default.t1_remote_mysql AS mysql('mysql1:3306', 'test', 't1_mysql_local', root, 'clickhouse')").find( - 'Database engine MySQL requested literal argument.') != -1 + with contextlib.closing(MySQLNodeInstance('root', 'clickhouse', '127.0.0.1', port=3308)) as mysql_node: + with pytest.raises(QueryRuntimeException) as exception: + mysql_node.query("CREATE DATABASE IF NOT EXISTS test_bad_arguments DEFAULT CHARACTER SET 'utf8'") + clickhouse_node.query("CREATE DATABASE test_database ENGINE = MySQL('mysql1:3306', test_bad_arguments, root, 'clickhouse')") + + assert 'Database engine MySQL requested literal argument.' in str(exception.value) + mysql_node.query("DROP DATABASE test_bad_arguments")