do not start mutation for alters with wrong type conversion

This commit is contained in:
Anton Popov 2021-02-20 16:05:33 +03:00
parent 04f5b72391
commit 48e188681c
3 changed files with 38 additions and 6 deletions

View File

@ -21,6 +21,7 @@
#include <Interpreters/PartLog.h>
#include <Interpreters/TreeRewriter.h>
#include <Interpreters/Context.h>
#include <Interpreters/inplaceBlockConversions.h>
#include <Parsers/ASTFunction.h>
#include <Parsers/ASTLiteral.h>
#include <Parsers/ASTNameTypePair.h>
@ -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<const ASTSetQuery &>().changes;

View File

@ -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

View File

@ -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;