From 094ce895da12c00e7bf57f8c0d9c0e7d9453517d Mon Sep 17 00:00:00 2001 From: zhang2014 Date: Mon, 13 Jul 2020 16:53:55 +0800 Subject: [PATCH] ISSUES-4006 fix test failure --- .../MySQL/MaterializeMySQLSyncThread.cpp | 2 +- .../MySQL/InterpretersMySQLDDLQuery.cpp | 64 +++++++++++++++---- .../tests/gtest_alter_command_parser.cpp | 2 +- .../MySQL/tests/gtest_alter_parser.cpp | 22 +++++++ 4 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 src/Parsers/MySQL/tests/gtest_alter_parser.cpp diff --git a/src/Databases/MySQL/MaterializeMySQLSyncThread.cpp b/src/Databases/MySQL/MaterializeMySQLSyncThread.cpp index 9adaaa2b3eb..2f648256514 100644 --- a/src/Databases/MySQL/MaterializeMySQLSyncThread.cpp +++ b/src/Databases/MySQL/MaterializeMySQLSyncThread.cpp @@ -92,7 +92,7 @@ MaterializeMySQLSyncThread::MaterializeMySQLSyncThread( , mysql_database_name(mysql_database_name_), pool(std::move(pool_)), client(std::move(client_)), settings(settings_) { /// TODO: 做简单的check, 失败即报错 - query_prefix = "EXTERNAL DDL FROM MySQL(" + backQuoteIfNeed(database_name) + ", " + backQuoteIfNeed(mysql_database_name) + ")"; + query_prefix = "EXTERNAL DDL FROM MySQL(" + backQuoteIfNeed(database_name) + ", " + backQuoteIfNeed(mysql_database_name) + ") "; startSynchronization(); } diff --git a/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp b/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp index 000e650317e..383fef04baf 100644 --- a/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp +++ b/src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -19,6 +20,7 @@ #include #include #include +#include namespace DB { @@ -363,15 +365,39 @@ ASTPtr InterpreterAlterImpl::getRewrittenQuery( if (alter_command->type == MySQLParser::ASTAlterCommand::ADD_COLUMN) { - const auto & additional_columns = getColumnsList(alter_command->additional_columns); + const auto & additional_columns_name_and_type = getColumnsList(alter_command->additional_columns); + const auto & additional_columns = InterpreterCreateQuery::formatColumns(additional_columns_name_and_type); - for (const auto & additional_column : InterpreterCreateQuery::formatColumns(additional_columns)->children) + String default_after_column; + for (size_t index = 0; index < additional_columns_name_and_type.size(); ++index) { auto rewritten_command = std::make_shared(); rewritten_command->type = ASTAlterCommand::ADD_COLUMN; rewritten_command->first = alter_command->first; - rewritten_command->col_decl = additional_column; - rewritten_command->column = std::make_shared(alter_command->column_name); + rewritten_command->col_decl = additional_columns->children[index]->clone(); + + if (!alter_command->column_name.empty()) + { + rewritten_command->column = std::make_shared(alter_command->column_name); + rewritten_command->children.push_back(rewritten_command->column); + } + else + { + if (default_after_column.empty()) + { + StoragePtr storage = DatabaseCatalog::instance().getTable({clickhouse_db, alter_query.table}, context); + Block storage_header = storage->getInMemoryMetadataPtr()->getSampleBlock(); + + /// Put the sign and version columns last + default_after_column = storage_header.getByPosition(storage_header.columns() - 3).name; + } + + rewritten_command->column = std::make_shared(default_after_column); + rewritten_command->children.push_back(rewritten_command->column); + default_after_column = rewritten_command->col_decl->as()->name; + } + + rewritten_command->children.push_back(rewritten_command->col_decl); rewritten_query->command_list->add(rewritten_command); } } @@ -384,11 +410,15 @@ ASTPtr InterpreterAlterImpl::getRewrittenQuery( } else if (alter_command->type == MySQLParser::ASTAlterCommand::RENAME_COLUMN) { - auto rewritten_command = std::make_shared(); - rewritten_command->type = ASTAlterCommand::RENAME_COLUMN; - rewritten_command->column = std::make_shared(alter_command->old_name); - rewritten_command->rename_to = std::make_shared(alter_command->column_name); - rewritten_query->command_list->add(rewritten_command); + if (alter_command->old_name != alter_command->column_name) + { + /// 'RENAME column_name TO column_name' is not allowed in Clickhouse + auto rewritten_command = std::make_shared(); + rewritten_command->type = ASTAlterCommand::RENAME_COLUMN; + rewritten_command->column = std::make_shared(alter_command->old_name); + rewritten_command->rename_to = std::make_shared(alter_command->column_name); + rewritten_query->command_list->add(rewritten_command); + } } else if (alter_command->type == MySQLParser::ASTAlterCommand::MODIFY_COLUMN) { @@ -398,18 +428,25 @@ ASTPtr InterpreterAlterImpl::getRewrittenQuery( auto rewritten_command = std::make_shared(); rewritten_command->type = ASTAlterCommand::MODIFY_COLUMN; rewritten_command->first = alter_command->first; - rewritten_command->column = std::make_shared(alter_command->column_name); - const auto & modify_columns = getColumnsList(alter_command->additional_columns); + auto modify_columns = getColumnsList(alter_command->additional_columns); if (modify_columns.size() != 1) throw Exception("It is a bug", ErrorCodes::LOGICAL_ERROR); new_column_name = modify_columns.front().name; + modify_columns.front().name = alter_command->old_name; rewritten_command->col_decl = InterpreterCreateQuery::formatColumns(modify_columns)->children[0]; + + if (!alter_command->column_name.empty()) + { + rewritten_command->column = std::make_shared(alter_command->column_name); + rewritten_command->children.push_back(rewritten_command->column); + } + rewritten_query->command_list->add(rewritten_command); } - if (!alter_command->old_name.empty()) + if (!alter_command->old_name.empty() && alter_command->old_name != new_column_name) { auto rewritten_command = std::make_shared(); rewritten_command->type = ASTAlterCommand::RENAME_COLUMN; @@ -420,6 +457,9 @@ ASTPtr InterpreterAlterImpl::getRewrittenQuery( } } + if (rewritten_query->command_list->commands.empty()) + return {}; + return rewritten_query; } diff --git a/src/Parsers/MySQL/tests/gtest_alter_command_parser.cpp b/src/Parsers/MySQL/tests/gtest_alter_command_parser.cpp index b9a0d8928e6..5534001241c 100644 --- a/src/Parsers/MySQL/tests/gtest_alter_command_parser.cpp +++ b/src/Parsers/MySQL/tests/gtest_alter_command_parser.cpp @@ -9,7 +9,7 @@ using namespace DB; using namespace DB::MySQLParser; -ASTPtr tryParserQuery(IParser & parser, const String & query) +static inline ASTPtr tryParserQuery(IParser & parser, const String & query) { return parseQuery(parser, query.data(), query.data() + query.size(), "", 0, 0); } diff --git a/src/Parsers/MySQL/tests/gtest_alter_parser.cpp b/src/Parsers/MySQL/tests/gtest_alter_parser.cpp new file mode 100644 index 00000000000..4ebbe332710 --- /dev/null +++ b/src/Parsers/MySQL/tests/gtest_alter_parser.cpp @@ -0,0 +1,22 @@ +#include + +#include +#include +#include + +using namespace DB; +using namespace DB::MySQLParser; + +static inline ASTPtr tryParserQuery(IParser & parser, const String & query) +{ + return parseQuery(parser, query.data(), query.data() + query.size(), "", 0, 0); +} + +TEST(ParserAlterQuery, AlterQuery) +{ + ParserAlterQuery alter_p; + + ASTPtr ast = tryParserQuery(alter_p, "ALTER TABLE test_table_2 ADD COLUMN (f INT, g INT)"); + EXPECT_EQ(ast->as()->command_list->children.size(), 1); + EXPECT_EQ(ast->as()->command_list->children[0]->as()->type, ASTAlterCommand::ADD_COLUMN); +}