diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index a0d23b8ab22..b09f068f509 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -1482,6 +1483,7 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S for (const auto & column : old_metadata.getColumns().getAllPhysical()) old_types.emplace(column.name, column.type.get()); + NamesAndTypesList columns_to_check_conversion; for (const AlterCommand & command : commands) { /// Just validate partition expression @@ -1571,9 +1573,9 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S throw Exception("ALTER of key column " + backQuoteIfNeed(command.column_name) + " is forbidden", ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN); - if (columns_alter_type_check_safe_for_partition.count(command.column_name)) + if (command.type == AlterCommand::MODIFY_COLUMN) { - if (command.type == AlterCommand::MODIFY_COLUMN) + if (columns_alter_type_check_safe_for_partition.count(command.column_name)) { auto it = old_types.find(command.column_name); @@ -1584,11 +1586,8 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S + " is not safe because it can change the representation of partition key", ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN); } - } - if (columns_alter_type_metadata_only.count(command.column_name)) - { - if (command.type == AlterCommand::MODIFY_COLUMN) + if (columns_alter_type_metadata_only.count(command.column_name)) { auto it = old_types.find(command.column_name); assert(it != old_types.end()); @@ -1598,6 +1597,12 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S + " is not safe because it can change the representation of primary key", ErrorCodes::ALTER_OF_COLUMN_IS_FORBIDDEN); } + + if (old_metadata.getColumns().has(command.column_name)) + { + columns_to_check_conversion.push_back( + new_metadata.getColumns().getPhysical(command.column_name)); + } } } } @@ -1605,6 +1610,12 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, const S checkProperties(new_metadata, old_metadata); checkTTLExpressions(new_metadata, old_metadata); + if (!columns_to_check_conversion.empty()) + { + auto old_header = old_metadata.getSampleBlock(); + performRequiredConversions(old_header, columns_to_check_conversion, global_context); + } + if (old_metadata.hasSettingsChanges()) { const auto current_changes = old_metadata.getSettingsChanges()->as().changes; diff --git a/tests/queries/0_stateless/01732_alters_bad_conversions.reference b/tests/queries/0_stateless/01732_alters_bad_conversions.reference new file mode 100644 index 00000000000..5f570c78579 --- /dev/null +++ b/tests/queries/0_stateless/01732_alters_bad_conversions.reference @@ -0,0 +1,4 @@ +CREATE TABLE default.bad_conversions\n(\n `a` UInt32\n)\nENGINE = MergeTree\nORDER BY tuple()\nSETTINGS index_granularity = 8192 +0 +CREATE TABLE default.bad_conversions_2\n(\n `e` Enum8(\'foo\' = 1, \'bar\' = 2)\n)\nENGINE = MergeTree\nORDER BY tuple()\nSETTINGS index_granularity = 8192 +0 diff --git a/tests/queries/0_stateless/01732_alters_bad_conversions.sql b/tests/queries/0_stateless/01732_alters_bad_conversions.sql new file mode 100644 index 00000000000..27da5242368 --- /dev/null +++ b/tests/queries/0_stateless/01732_alters_bad_conversions.sql @@ -0,0 +1,17 @@ +DROP TABLE IF EXISTS bad_conversions; +DROP TABLE IF EXISTS bad_conversions_2; + +CREATE TABLE bad_conversions (a UInt32) ENGINE = MergeTree ORDER BY tuple(); +INSERT INTO bad_conversions VALUES (1); +ALTER TABLE bad_conversions MODIFY COLUMN a Array(String); -- { serverError 53 } +SHOW CREATE TABLE bad_conversions; +SELECT count() FROM system.mutations WHERE table = 'bad_conversions' AND database = currentDatabase(); + +CREATE TABLE bad_conversions_2 (e Enum('foo' = 1, 'bar' = 2)) ENGINE = MergeTree ORDER BY tuple(); +INSERT INTO bad_conversions_2 VALUES (1); +ALTER TABLE bad_conversions_2 MODIFY COLUMN e Enum('bar' = 1, 'foo' = 2); -- { serverError 70 } +SHOW CREATE TABLE bad_conversions_2; +SELECT count() FROM system.mutations WHERE table = 'bad_conversions_2' AND database = currentDatabase(); + +DROP TABLE IF EXISTS bad_conversions; +DROP TABLE IF EXISTS bad_conversions_2;