From b8b0f1c3d3a2df7d0d23e4f67d6f6afe3c9dc5ca Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 2 Feb 2023 21:11:19 +0100 Subject: [PATCH] Make RENAME COLUMN barrier --- .../MergeTree/MergeTreeMarksLoader.cpp | 16 +++++++++- .../MergeTree/ReplicatedMergeTreeQueue.cpp | 25 ++++++++++++++- src/Storages/StorageMergeTree.cpp | 31 +++++++++++++++++-- .../02543_alter_rename_modify_stuck.sh | 2 +- 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/Storages/MergeTree/MergeTreeMarksLoader.cpp b/src/Storages/MergeTree/MergeTreeMarksLoader.cpp index 3fc7ff54c35..1d85ac1bd34 100644 --- a/src/Storages/MergeTree/MergeTreeMarksLoader.cpp +++ b/src/Storages/MergeTree/MergeTreeMarksLoader.cpp @@ -102,6 +102,15 @@ MarkCache::MappedPtr MergeTreeMarksLoader::loadMarksImpl() auto res = std::make_shared(marks_count * columns_in_mark); + if (file_size == 0 && marks_count != 0) + { + throw Exception( + ErrorCodes::CORRUPTED_DATA, + "Empty marks file '{}': {}, must be: {}", + std::string(fs::path(data_part_storage->getFullPath()) / mrk_path), + file_size, expected_uncompressed_size); + } + if (!index_granularity_info.mark_type.compressed && expected_uncompressed_size != file_size) throw Exception( ErrorCodes::CORRUPTED_DATA, @@ -138,7 +147,12 @@ MarkCache::MappedPtr MergeTreeMarksLoader::loadMarksImpl() } if (i * mark_size != expected_uncompressed_size) - throw Exception(ErrorCodes::CANNOT_READ_ALL_DATA, "Cannot read all marks from file {}", mrk_path); + { + throw Exception( + ErrorCodes::CANNOT_READ_ALL_DATA, + "Cannot read all marks from file {}, marks expected {} (bytes size {}), marks read {} (bytes size {})", + mrk_path, marks_count, expected_uncompressed_size, i, reader->count()); + } } res->protect(); diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp index e8ed0888fa1..2c957132918 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp @@ -1813,7 +1813,30 @@ MutationCommands ReplicatedMergeTreeQueue::getMutationCommands( MutationCommands commands; for (auto it = begin; it != end; ++it) - commands.insert(commands.end(), it->second->entry->commands.begin(), it->second->entry->commands.end()); + { + bool rename_command = false; + if (it->second->entry->isAlterMutation()) + { + const auto & single_mutation_commands = it->second->entry->commands; + for (const auto & command : single_mutation_commands) + { + if (command.type == MutationCommand::Type::RENAME_COLUMN) + { + rename_command = true; + break; + } + } + } + + if (rename_command) + { + if (commands.empty()) + commands.insert(commands.end(), it->second->entry->commands.begin(), it->second->entry->commands.end()); + break; + } + else + commands.insert(commands.end(), it->second->entry->commands.begin(), it->second->entry->commands.end()); + } return commands; } diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp index a9664007614..73585a630cb 100644 --- a/src/Storages/StorageMergeTree.cpp +++ b/src/Storages/StorageMergeTree.cpp @@ -1114,9 +1114,34 @@ MergeMutateSelectedEntryPtr StorageMergeTree::selectPartsToMutate( if (current_ast_elements + commands_size >= max_ast_elements) break; - current_ast_elements += commands_size; - commands->insert(commands->end(), it->second.commands.begin(), it->second.commands.end()); - last_mutation_to_apply = it; + bool rename_command = false; + const auto & single_mutation_commands = it->second.commands; + for (const auto & command : single_mutation_commands) + { + if (command.type == MutationCommand::Type::RENAME_COLUMN) + { + rename_command = true; + break; + } + } + + if (rename_command) + { + if (commands->empty()) + { + current_ast_elements += commands_size; + commands->insert(commands->end(), it->second.commands.begin(), it->second.commands.end()); + last_mutation_to_apply = it; + } + break; + } + else + { + current_ast_elements += commands_size; + commands->insert(commands->end(), it->second.commands.begin(), it->second.commands.end()); + last_mutation_to_apply = it; + } + } assert(commands->empty() == (last_mutation_to_apply == mutations_end_it)); diff --git a/tests/queries/0_stateless/02543_alter_rename_modify_stuck.sh b/tests/queries/0_stateless/02543_alter_rename_modify_stuck.sh index 0e9b39d6dae..adaf1846552 100755 --- a/tests/queries/0_stateless/02543_alter_rename_modify_stuck.sh +++ b/tests/queries/0_stateless/02543_alter_rename_modify_stuck.sh @@ -32,7 +32,7 @@ while [[ $counter -lt $retries ]]; do done -$CLICKHOUSE_CLIENT --query="ALTER TABLE table_to_rename UPDATE v2 = 77 WHERE 1 = 1" & +$CLICKHOUSE_CLIENT --query="ALTER TABLE table_to_rename UPDATE v2 = 77 WHERE 1 = 1 SETTINGS mutations_sync = 2" & counter=0 retries=60