From d9e6789f781693261eeec06cd28f49b314fe0b81 Mon Sep 17 00:00:00 2001 From: Sabyanin Maxim Date: Thu, 15 Nov 2018 16:12:27 +0300 Subject: [PATCH] make MergeTree work with comments --- dbms/src/Storages/AlterCommands.cpp | 11 +++ dbms/src/Storages/AlterCommands.h | 1 + dbms/src/Storages/MergeTree/MergeTreeData.cpp | 2 +- dbms/src/Storages/StorageMergeTree.cpp | 10 +++ .../00725_comment_columns.reference | 66 +++++++++-------- .../0_stateless/00725_comment_columns.sql | 70 +++++++++++-------- 6 files changed, 103 insertions(+), 57 deletions(-) diff --git a/dbms/src/Storages/AlterCommands.cpp b/dbms/src/Storages/AlterCommands.cpp index 3c75f806ad1..6748f56a5ab 100644 --- a/dbms/src/Storages/AlterCommands.cpp +++ b/dbms/src/Storages/AlterCommands.cpp @@ -477,4 +477,15 @@ void AlterCommands::validate(const IStorage & table, const Context & context) } } +bool AlterCommands::is_mutable() const +{ + for (const auto & param : *this) + { + if (param.is_mutable()) + return true; + } + + return false; +} + } diff --git a/dbms/src/Storages/AlterCommands.h b/dbms/src/Storages/AlterCommands.h index ac6d74d249a..e870e2f9762 100644 --- a/dbms/src/Storages/AlterCommands.h +++ b/dbms/src/Storages/AlterCommands.h @@ -70,6 +70,7 @@ public: void apply(ColumnsDescription & columns_description) const; void validate(const IStorage & table, const Context & context); + bool is_mutable() const; }; } diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index 012991db465..a9126dc79a9 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -908,7 +908,7 @@ void MergeTreeData::checkAlter(const AlterCommands & commands) for (const AlterCommand & command : commands) { - if (command.type == AlterCommand::COMMENT_COLUMN) + if (!command.is_mutable()) { continue; } diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index 0e926218a05..2686427fa74 100644 --- a/dbms/src/Storages/StorageMergeTree.cpp +++ b/dbms/src/Storages/StorageMergeTree.cpp @@ -188,6 +188,16 @@ void StorageMergeTree::alter( const String & table_name, const Context & context) { + if (!params.is_mutable()) + { + auto table_soft_lock = lockStructureForAlter(__PRETTY_FUNCTION__); + auto new_columns = getColumns(); + params.apply(new_columns); + context.getDatabase(database_name)->alterTable(context, table_name, new_columns, {}); + setColumns(std::move(new_columns)); + return; + } + /// NOTE: Here, as in ReplicatedMergeTree, you can do ALTER which does not block the writing of data for a long time. auto merge_blocker = merger_mutator.actions_blocker.cancel(); diff --git a/dbms/tests/queries/0_stateless/00725_comment_columns.reference b/dbms/tests/queries/0_stateless/00725_comment_columns.reference index 19dac99b721..ca4edfb3122 100644 --- a/dbms/tests/queries/0_stateless/00725_comment_columns.reference +++ b/dbms/tests/queries/0_stateless/00725_comment_columns.reference @@ -1,28 +1,38 @@ -CREATE TABLE test.check_query_comment_column ( first_column UInt8 DEFAULT 1 COMMENT \'first comment\', fourth_column UInt8 COMMENT \'fourth comment\', fifth_column UInt8, second_column UInt8 MATERIALIZED first_column COMMENT \'second comment\', third_column UInt8 ALIAS second_column COMMENT \'third comment\') ENGINE = TinyLog -┌─table──────────────────────┬─name──────────┬─comment────────┐ -│ check_query_comment_column │ first_column │ first comment │ -│ check_query_comment_column │ fourth_column │ fourth comment │ -│ check_query_comment_column │ fifth_column │ │ -│ check_query_comment_column │ second_column │ second comment │ -│ check_query_comment_column │ third_column │ third comment │ -└────────────────────────────┴───────────────┴────────────────┘ -CREATE TABLE test.check_query_comment_column ( first_column UInt8 DEFAULT 1 COMMENT \'another first column\', fourth_column UInt8 COMMENT \'another fourth column\', fifth_column UInt8 COMMENT \'another fifth column\', second_column UInt8 MATERIALIZED first_column COMMENT \'another second column\', third_column UInt8 ALIAS second_column COMMENT \'another third column\') ENGINE = TinyLog -┌─table──────────────────────┬─name──────────┬─comment───────────────┐ -│ check_query_comment_column │ first_column │ another first column │ -│ check_query_comment_column │ fourth_column │ another fourth column │ -│ check_query_comment_column │ fifth_column │ another fifth column │ -│ check_query_comment_column │ second_column │ another second column │ -│ check_query_comment_column │ third_column │ another third column │ -└────────────────────────────┴───────────────┴───────────────────────┘ -CREATE TABLE test.check_query_comment_column ( first_column Date COMMENT \'first comment\', second_column UInt8 COMMENT \'second comment\', third_column UInt8 COMMENT \'third comment\') ENGINE = MergeTree(first_column, (second_column, second_column), 8192) -┌─table──────────────────────┬─name──────────┬─comment────────┐ -│ check_query_comment_column │ first_column │ first comment │ -│ check_query_comment_column │ second_column │ second comment │ -│ check_query_comment_column │ third_column │ third comment │ -└────────────────────────────┴───────────────┴────────────────┘ -CREATE TABLE test.check_query_comment_column ( first_column Date COMMENT \'another first comment\', second_column UInt8 COMMENT \'another second comment\', third_column UInt8 COMMENT \'another third comment\') ENGINE = MergeTree(first_column, (second_column, second_column), 8192) -┌─table──────────────────────┬─name──────────┬─comment────────────────┐ -│ check_query_comment_column │ first_column │ another first comment │ -│ check_query_comment_column │ second_column │ another second comment │ -│ check_query_comment_column │ third_column │ another third comment │ -└────────────────────────────┴───────────────┴────────────────────────┘ +CREATE TABLE test.check_query_comment_column ( first_column UInt8 DEFAULT 1 COMMENT \'comment 1\', fourth_column UInt8 COMMENT \'comment 4\', fifth_column UInt8, second_column UInt8 MATERIALIZED first_column COMMENT \'comment 2\', third_column UInt8 ALIAS second_column COMMENT \'comment 3\') ENGINE = TinyLog +first_column UInt8 DEFAULT 1 comment 1 +fourth_column UInt8 comment 4 +fifth_column UInt8 +second_column UInt8 MATERIALIZED first_column comment 2 +third_column UInt8 ALIAS second_column comment 3 +┌─table──────────────────────┬─name──────────┬─comment───┐ +│ check_query_comment_column │ first_column │ comment 1 │ +│ check_query_comment_column │ fourth_column │ comment 4 │ +│ check_query_comment_column │ fifth_column │ │ +│ check_query_comment_column │ second_column │ comment 2 │ +│ check_query_comment_column │ third_column │ comment 3 │ +└────────────────────────────┴───────────────┴───────────┘ +CREATE TABLE test.check_query_comment_column ( first_column UInt8 DEFAULT 1 COMMENT \'comment 1_1\', fourth_column UInt8 COMMENT \'comment 4_1\', fifth_column UInt8 COMMENT \'comment 5_1\', second_column UInt8 MATERIALIZED first_column COMMENT \'comment 2_1\', third_column UInt8 ALIAS second_column COMMENT \'comment 3_1\') ENGINE = TinyLog +┌─table──────────────────────┬─name──────────┬─comment─────┐ +│ check_query_comment_column │ first_column │ comment 1_2 │ +│ check_query_comment_column │ fourth_column │ comment 4_2 │ +│ check_query_comment_column │ fifth_column │ comment 5_2 │ +│ check_query_comment_column │ second_column │ comment 2_2 │ +│ check_query_comment_column │ third_column │ comment 3_2 │ +└────────────────────────────┴───────────────┴─────────────┘ +CREATE TABLE test.check_query_comment_column ( first_column UInt8 DEFAULT 1 COMMENT \'comment 1_2\', fourth_column UInt8 COMMENT \'comment 4_2\', fifth_column UInt8 COMMENT \'comment 5_2\', second_column UInt8 MATERIALIZED first_column COMMENT \'comment 2_2\', third_column UInt8 ALIAS second_column COMMENT \'comment 3_2\') ENGINE = TinyLog +CREATE TABLE test.check_query_comment_column ( first_column UInt8 COMMENT \'comment 1\', second_column UInt8 COMMENT \'comment 2\', third_column UInt8 COMMENT \'comment 3\') ENGINE = MergeTree() PARTITION BY second_column ORDER BY first_column SAMPLE BY first_column SETTINGS index_granularity = 8192 +first_column UInt8 comment 1 +second_column UInt8 comment 2 +third_column UInt8 comment 3 +┌─table──────────────────────┬─name──────────┬─comment───┐ +│ check_query_comment_column │ first_column │ comment 1 │ +│ check_query_comment_column │ second_column │ comment 2 │ +│ check_query_comment_column │ third_column │ comment 3 │ +└────────────────────────────┴───────────────┴───────────┘ +CREATE TABLE test.check_query_comment_column ( first_column UInt8 COMMENT \'comment 1_2\', second_column UInt8 COMMENT \'comment 2_2\', third_column UInt8 COMMENT \'comment 3_2\') ENGINE = MergeTree() PARTITION BY second_column ORDER BY first_column SAMPLE BY first_column SETTINGS index_granularity = 8192 +CREATE TABLE test.check_query_comment_column ( first_column UInt8 COMMENT \'comment 1_3\', second_column UInt8 COMMENT \'comment 2_3\', third_column UInt8 COMMENT \'comment 3_3\') ENGINE = MergeTree() PARTITION BY second_column ORDER BY first_column SAMPLE BY first_column SETTINGS index_granularity = 8192 +┌─table──────────────────────┬─name──────────┬─comment─────┐ +│ check_query_comment_column │ first_column │ comment 1_3 │ +│ check_query_comment_column │ second_column │ comment 2_3 │ +│ check_query_comment_column │ third_column │ comment 3_3 │ +└────────────────────────────┴───────────────┴─────────────┘ diff --git a/dbms/tests/queries/0_stateless/00725_comment_columns.sql b/dbms/tests/queries/0_stateless/00725_comment_columns.sql index ada4e4e1983..97667616983 100644 --- a/dbms/tests/queries/0_stateless/00725_comment_columns.sql +++ b/dbms/tests/queries/0_stateless/00725_comment_columns.sql @@ -1,16 +1,18 @@ CREATE DATABASE IF NOT EXISTS test; DROP TABLE IF EXISTS test.check_query_comment_column; +-- Check COMMENT COLUMN and MODIFY COLUMN statements with simple engine CREATE TABLE test.check_query_comment_column ( - first_column UInt8 DEFAULT 1 COMMENT 'first comment', - second_column UInt8 MATERIALIZED first_column COMMENT 'second comment', - third_column UInt8 ALIAS second_column COMMENT 'third comment', - fourth_column UInt8 COMMENT 'fourth comment', + first_column UInt8 DEFAULT 1 COMMENT 'comment 1', + second_column UInt8 MATERIALIZED first_column COMMENT 'comment 2', + third_column UInt8 ALIAS second_column COMMENT 'comment 3', + fourth_column UInt8 COMMENT 'comment 4', fifth_column UInt8 ) ENGINE = TinyLog; SHOW CREATE TABLE test.check_query_comment_column; +DESCRIBE TABLE test.check_query_comment_column; SELECT table, name, comment FROM system.columns @@ -18,30 +20,42 @@ WHERE table = 'check_query_comment_column' AND database = 'test' FORMAT PrettyCompactNoEscapes; ALTER TABLE test.check_query_comment_column - COMMENT COLUMN first_column 'another first column', - COMMENT COLUMN second_column 'another second column', - COMMENT COLUMN third_column 'another third column', - COMMENT COLUMN fourth_column 'another fourth column', - COMMENT COLUMN fifth_column 'another fifth column'; + COMMENT COLUMN first_column 'comment 1_1', + COMMENT COLUMN second_column 'comment 2_1', + COMMENT COLUMN third_column 'comment 3_1', + COMMENT COLUMN fourth_column 'comment 4_1', + COMMENT COLUMN fifth_column 'comment 5_1'; SHOW CREATE TABLE test.check_query_comment_column; +ALTER TABLE test.check_query_comment_column + MODIFY COLUMN first_column COMMENT 'comment 1_2', + MODIFY COLUMN second_column COMMENT 'comment 2_2', + MODIFY COLUMN third_column COMMENT 'comment 3_2', + MODIFY COLUMN fourth_column COMMENT 'comment 4_2', + MODIFY COLUMN fifth_column COMMENT 'comment 5_2'; + SELECT table, name, comment FROM system.columns WHERE table = 'check_query_comment_column' AND database = 'test' FORMAT PrettyCompactNoEscapes; +SHOW CREATE TABLE test.check_query_comment_column; DROP TABLE IF EXISTS test.check_query_comment_column; - +-- Check `ALTER TABLE table_name COMMENT COLUMN 'comment'` statement with MergeTree engine CREATE TABLE test.check_query_comment_column ( - first_column Date COMMENT 'first comment', - second_column UInt8 COMMENT 'second comment', - third_column UInt8 COMMENT 'third comment' - ) ENGINE = MergeTree(first_column, (second_column, second_column), 8192); + first_column UInt8 COMMENT 'comment 1', + second_column UInt8 COMMENT 'comment 2', + third_column UInt8 COMMENT 'comment 3' + ) ENGINE = MergeTree() + ORDER BY first_column + PARTITION BY second_column + SAMPLE BY first_column; SHOW CREATE TABLE test.check_query_comment_column; +DESCRIBE TABLE test.check_query_comment_column; SELECT table, name, comment FROM system.columns @@ -49,25 +63,23 @@ WHERE table = 'check_query_comment_column' AND database = 'test' FORMAT PrettyCompactNoEscapes; ALTER TABLE test.check_query_comment_column - COMMENT COLUMN first_column 'another first comment', - COMMENT COLUMN second_column 'another second comment', - COMMENT COLUMN third_column 'another third comment'; + COMMENT COLUMN first_column 'comment 1_2', + COMMENT COLUMN second_column 'comment 2_2', + COMMENT COLUMN third_column 'comment 3_2'; SHOW CREATE TABLE test.check_query_comment_column; -SELECT table, name, comment -FROM system.columns -WHERE table = 'check_query_comment_column' and database = 'test' -FORMAT PrettyCompactNoEscapes; +ALTER TABLE test.check_query_comment_column + MODIFY COLUMN first_column COMMENT 'comment 1_3', + MODIFY COLUMN second_column COMMENT 'comment 2_3', + MODIFY COLUMN third_column COMMENT 'comment 3_3'; -DROP TABLE IF test.check_query_comment_column; +SHOW CREATE TABLE test.check_query_comment_column; -CREATE TABLE test.check_query_comment_column - ( - first_column UInt8 COMMENT 'first comment' - ) ENGINE = TinyLog; - -ALTER TABLE test.check_query_comment_column MODIFY COLUMN first_column COMMENT 'another comment'; +ALTER TABLE test.check_query_comment_column + MODIFY COLUMN first_column DEFAULT 1 COMMENT 'comment 1_3', + MODIFY COLUMN second_column COMMENT 'comment 2_3', -- We can't change default value of partition key. + MODIFY COLUMN third_column DEFAULT 1 COMMENT 'comment 3_3'; SELECT table, name, comment FROM system.columns @@ -75,3 +87,5 @@ WHERE table = 'check_query_comment_column' and database = 'test' FORMAT PrettyCompactNoEscapes; DROP TABLE IF EXISTS test.check_query_comment_column; + +-- TODO: add here tests with ReplicatedMergeTree