diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index e59078c642c..845aae52582 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -785,7 +785,7 @@ bool AlterCommand::isRequireMutationStage(const StorageInMemoryMetadata & metada /// Drop alias is metadata alter, in other case mutation is required. if (type == DROP_COLUMN) - return metadata.columns.hasPhysical(column_name); + return metadata.columns.hasColumnOrNested(GetColumnsOptions::AllPhysical, column_name); if (type != MODIFY_COLUMN || data_type == nullptr) return false; diff --git a/src/Storages/ColumnsDescription.cpp b/src/Storages/ColumnsDescription.cpp index 1264da77b04..3aa5b28fed5 100644 --- a/src/Storages/ColumnsDescription.cpp +++ b/src/Storages/ColumnsDescription.cpp @@ -397,6 +397,15 @@ NamesAndTypesList ColumnsDescription::getSubcolumns(const String & name_in_stora return NamesAndTypesList(range.first, range.second); } +NamesAndTypesList ColumnsDescription::getNested(const String & column_name) const +{ + auto range = getNameRange(columns, column_name); + NamesAndTypesList nested; + for (auto & it = range.first; it != range.second; ++it) + nested.emplace_back(it->name, it->type); + return nested; +} + void ColumnsDescription::addSubcolumnsToList(NamesAndTypesList & source_list) const { NamesAndTypesList subcolumns_list; @@ -603,6 +612,13 @@ bool ColumnsDescription::hasColumnOrSubcolumn(GetColumnsOptions::Kind kind, cons || hasSubcolumn(column_name); } +bool ColumnsDescription::hasColumnOrNested(GetColumnsOptions::Kind kind, const String & column_name) const +{ + auto range = getNameRange(columns, column_name); + return range.first != range.second && + defaultKindToGetKind(range.first->default_desc.kind) & kind; +} + bool ColumnsDescription::hasDefaults() const { for (const auto & column : columns) diff --git a/src/Storages/ColumnsDescription.h b/src/Storages/ColumnsDescription.h index 670aeaa293d..75db8b92545 100644 --- a/src/Storages/ColumnsDescription.h +++ b/src/Storages/ColumnsDescription.h @@ -127,7 +127,10 @@ public: NamesAndTypesList getEphemeral() const; NamesAndTypesList getAllPhysical() const; /// ordinary + materialized. NamesAndTypesList getAll() const; /// ordinary + materialized + aliases + ephemeral + /// Returns .size0/.null/... NamesAndTypesList getSubcolumns(const String & name_in_storage) const; + /// Returns column_name.* + NamesAndTypesList getNested(const String & column_name) const; using ColumnTTLs = std::unordered_map; ColumnTTLs getColumnTTLs() const; @@ -167,6 +170,7 @@ public: bool hasPhysical(const String & column_name) const; bool hasColumnOrSubcolumn(GetColumnsOptions::Kind kind, const String & column_name) const; + bool hasColumnOrNested(GetColumnsOptions::Kind kind, const String & column_name) const; NameAndTypePair getPhysical(const String & column_name) const; NameAndTypePair getColumnOrSubcolumn(GetColumnsOptions::Kind kind, const String & column_name) const; diff --git a/src/Storages/MergeTree/MutateTask.cpp b/src/Storages/MergeTree/MutateTask.cpp index c71d2a89e19..aac315e8f25 100644 --- a/src/Storages/MergeTree/MutateTask.cpp +++ b/src/Storages/MergeTree/MutateTask.cpp @@ -80,20 +80,28 @@ static void splitMutationCommands( { for_file_renames.push_back(command); } - else if (part_columns.has(command.column_name)) + else if (bool has_column = part_columns.has(command.column_name), has_nested_column = part_columns.hasNested(command.column_name); has_column || has_nested_column) { - if (command.type == MutationCommand::Type::DROP_COLUMN) + if (command.type == MutationCommand::Type::DROP_COLUMN || command.type == MutationCommand::Type::RENAME_COLUMN) { - mutated_columns.emplace(command.column_name); + if (has_nested_column) + { + const auto & nested = part_columns.getNested(command.column_name); + assert(!nested.empty()); + for (const auto & nested_column : nested) + mutated_columns.emplace(nested_column.name); + } + else + mutated_columns.emplace(command.column_name); } - else if (command.type == MutationCommand::Type::RENAME_COLUMN) + + if (command.type == MutationCommand::Type::RENAME_COLUMN) { for_interpreter.push_back( { .type = MutationCommand::Type::READ_COLUMN, .column_name = command.rename_to, }); - mutated_columns.emplace(command.column_name); part_columns.rename(command.column_name, command.rename_to); } } diff --git a/tests/queries/0_stateless/02260_alter_compact_part_drop_nested_column.reference b/tests/queries/0_stateless/02260_alter_compact_part_drop_nested_column.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02260_alter_compact_part_drop_nested_column.sql.j2 b/tests/queries/0_stateless/02260_alter_compact_part_drop_nested_column.sql.j2 new file mode 100644 index 00000000000..57db8618f8d --- /dev/null +++ b/tests/queries/0_stateless/02260_alter_compact_part_drop_nested_column.sql.j2 @@ -0,0 +1,17 @@ +{# force compact parts and wide #} +{% for min_bytes_for_wide_part in [100000, 0] %} +DROP TABLE IF EXISTS compact_alter_{{ min_bytes_for_wide_part }}; +CREATE TABLE compact_alter_{{ min_bytes_for_wide_part }} (d Date, s String, k UInt64) ENGINE=MergeTree() PARTITION BY d ORDER BY k SETTINGS min_bytes_for_wide_part={{ min_bytes_for_wide_part }}; + +INSERT INTO compact_alter_{{ min_bytes_for_wide_part }} VALUES ('2015-01-01', '2015-01-01 00:00:00', 10); + +ALTER TABLE compact_alter_{{ min_bytes_for_wide_part }} ADD COLUMN n.d Int; +-- force columns creation +OPTIMIZE TABLE compact_alter_{{ min_bytes_for_wide_part }} FINAL; +-- this command will not drop n.d from compact part columns.txt +ALTER TABLE compact_alter_{{ min_bytes_for_wide_part }} DROP COLUMN n; +-- and now modify column will trigger READ_COLUMN of n.d, and it will bail +ALTER TABLE compact_alter_{{ min_bytes_for_wide_part }} MODIFY COLUMN s DateTime('UTC') DEFAULT '1970-01-01 00:00:00'; + +DROP TABLE compact_alter_{{ min_bytes_for_wide_part }}; +{% endfor %}