From 7b35920d4cbc368759e9185f7b5a980bfcc22403 Mon Sep 17 00:00:00 2001 From: alesapin Date: Sun, 3 Apr 2022 14:03:34 +0200 Subject: [PATCH 1/2] Make more alters of nested types metadata-only --- src/Storages/AlterCommands.cpp | 33 ++++++++++++------- .../02251_alter_enum_nested_struct.reference | 7 ++++ .../02251_alter_enum_nested_struct.sql | 27 +++++++++++++++ 3 files changed, 55 insertions(+), 12 deletions(-) create mode 100644 tests/queries/0_stateless/02251_alter_enum_nested_struct.reference create mode 100644 tests/queries/0_stateless/02251_alter_enum_nested_struct.sql diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 44f208adacc..16e1f044fd9 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -696,20 +696,22 @@ namespace /// The function works for Arrays and Nullables of the same structure. bool isMetadataOnlyConversion(const IDataType * from, const IDataType * to) { - if (from->equals(*to)) - return true; - - if (const auto * from_enum8 = typeid_cast(from)) + auto is_compatible_enum_types_conversion = [](const IDataType * from_type, const IDataType * to_type) { - if (const auto * to_enum8 = typeid_cast(to)) - return to_enum8->contains(*from_enum8); - } + if (const auto * from_enum8 = typeid_cast(from_type)) + { + if (const auto * to_enum8 = typeid_cast(to_type)) + return to_enum8->contains(*from_enum8); + } - if (const auto * from_enum16 = typeid_cast(from)) - { - if (const auto * to_enum16 = typeid_cast(to)) - return to_enum16->contains(*from_enum16); - } + if (const auto * from_enum16 = typeid_cast(from_type)) + { + if (const auto * to_enum16 = typeid_cast(to_type)) + return to_enum16->contains(*from_enum16); + } + + return false; + }; static const std::unordered_multimap ALLOWED_CONVERSIONS = { @@ -721,11 +723,18 @@ bool isMetadataOnlyConversion(const IDataType * from, const IDataType * to) { typeid(DataTypeUInt16), typeid(DataTypeDate) }, }; + /// Unwrap some nested and check for valid conevrsions while (true) { + /// types are equal, obviously pure metadata alter if (from->equals(*to)) return true; + /// We just adding something to enum, nothing changed on disk + if (is_compatible_enum_types_conversion(from, to)) + return true; + + /// Types changed, but representation on disk didn't auto it_range = ALLOWED_CONVERSIONS.equal_range(typeid(*from)); for (auto it = it_range.first; it != it_range.second; ++it) { diff --git a/tests/queries/0_stateless/02251_alter_enum_nested_struct.reference b/tests/queries/0_stateless/02251_alter_enum_nested_struct.reference new file mode 100644 index 00000000000..ada5f47c230 --- /dev/null +++ b/tests/queries/0_stateless/02251_alter_enum_nested_struct.reference @@ -0,0 +1,7 @@ +1 ['Option2','Option1'] +2 ['Option1'] +3 ['Option1','Option3'] +1 ['Option2','Option1'] +2 ['Option1'] +3 ['Option1','Option3'] +0 diff --git a/tests/queries/0_stateless/02251_alter_enum_nested_struct.sql b/tests/queries/0_stateless/02251_alter_enum_nested_struct.sql new file mode 100644 index 00000000000..ad2dab3631f --- /dev/null +++ b/tests/queries/0_stateless/02251_alter_enum_nested_struct.sql @@ -0,0 +1,27 @@ +DROP TABLE IF EXISTS alter_enum_array; + +CREATE TABLE alter_enum_array( + Key UInt64, + Value Array(Enum8('Option1'=1, 'Option2'=2)) +) +ENGINE=MergeTree() +ORDER BY tuple(); + +INSERT INTO alter_enum_array VALUES (1, ['Option2', 'Option1']), (2, ['Option1']); + +ALTER TABLE alter_enum_array MODIFY COLUMN Value Array(Enum8('Option1'=1, 'Option2'=2, 'Option3'=3)) SETTINGS mutations_sync=2; + +INSERT INTO alter_enum_array VALUES (3, ['Option1','Option3']); + +SELECT * FROM alter_enum_array ORDER BY Key; + +DETACH TABLE alter_enum_array; +ATTACH TABLE alter_enum_array; + +SELECT * FROM alter_enum_array ORDER BY Key; + +OPTIMIZE TABLE alter_enum_array FINAL; + +SELECT COUNT() FROM system.mutations where table='alter_enum_array' and database=currentDatabase(); + +DROP TABLE IF EXISTS alter_enum_array; From 0477e74f42b9d2cc9056574b01f02e82016a0a52 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 4 Apr 2022 19:41:54 +0200 Subject: [PATCH 2/2] Get rid of caps --- src/Storages/AlterCommands.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 16e1f044fd9..15095335a51 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -713,7 +713,7 @@ bool isMetadataOnlyConversion(const IDataType * from, const IDataType * to) return false; }; - static const std::unordered_multimap ALLOWED_CONVERSIONS = + static const std::unordered_multimap allowed_conversions = { { typeid(DataTypeEnum8), typeid(DataTypeInt8) }, { typeid(DataTypeEnum16), typeid(DataTypeInt16) }, @@ -735,7 +735,7 @@ bool isMetadataOnlyConversion(const IDataType * from, const IDataType * to) return true; /// Types changed, but representation on disk didn't - auto it_range = ALLOWED_CONVERSIONS.equal_range(typeid(*from)); + auto it_range = allowed_conversions.equal_range(typeid(*from)); for (auto it = it_range.first; it != it_range.second; ++it) { if (it->second == typeid(*to))