From 77edd41b2e0e2a8e9d89541eb3bf6a8323aaedbb Mon Sep 17 00:00:00 2001 From: jewisliu Date: Wed, 6 Apr 2022 15:23:18 +0800 Subject: [PATCH] [Improvement] improvement in PARTITION ALL 1. ASTPartition::formatImpl should output ALL while executing ALTER TABLE t DETACH PARTITION ALL 2. prohibit PARTITION ALL excepte DETACH PARTITION ALL --- src/Parsers/ASTPartition.cpp | 13 +++++++---- src/Parsers/ParserPartition.cpp | 4 ---- src/Storages/MergeTree/MergeTreeData.cpp | 20 +++++++++++----- src/Storages/StorageMergeTree.cpp | 5 ++-- .../0_stateless/00753_alter_attach.sql | 23 +++++++++++++++++++ 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/Parsers/ASTPartition.cpp b/src/Parsers/ASTPartition.cpp index 06bfe4f5217..87d159b5817 100644 --- a/src/Parsers/ASTPartition.cpp +++ b/src/Parsers/ASTPartition.cpp @@ -35,10 +35,15 @@ void ASTPartition::formatImpl(const FormatSettings & settings, FormatState & sta } else { - settings.ostr << (settings.hilite ? hilite_keyword : "") << "ID " << (settings.hilite ? hilite_none : ""); - WriteBufferFromOwnString id_buf; - writeQuoted(id, id_buf); - settings.ostr << id_buf.str(); + if (all) + settings.ostr << "ALL"; + else + { + settings.ostr << (settings.hilite ? hilite_keyword : "") << "ID " << (settings.hilite ? hilite_none : ""); + WriteBufferFromOwnString id_buf; + writeQuoted(id, id_buf); + settings.ostr << id_buf.str(); + } } } diff --git a/src/Parsers/ParserPartition.cpp b/src/Parsers/ParserPartition.cpp index 5af442826df..9f1d4d4e889 100644 --- a/src/Parsers/ParserPartition.cpp +++ b/src/Parsers/ParserPartition.cpp @@ -32,10 +32,6 @@ bool ParserPartition::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) } else if (s_all.ignore(pos, expected)) { - ASTPtr value = makeASTFunction("tuple"); - partition->value = value; - partition->children.push_back(value); - partition->fields_count = 0; partition->all = true; } else diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index eacec8f50e5..478c1570a23 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -3385,7 +3385,14 @@ void MergeTreeData::checkAlterPartitionIsPossible( else { /// We are able to parse it - getPartitionIDFromQuery(command.partition, getContext()); + const auto * partition_ast = command.partition->as(); + if (partition_ast && partition_ast->all) + { + if (command.type != PartitionCommand::DROP_PARTITION) + throw DB::Exception("Only support DETACH PARTITION ALL currently", ErrorCodes::SUPPORT_IS_DISABLED); + } + else + getPartitionIDFromQuery(command.partition, getContext()); } } } @@ -3393,14 +3400,15 @@ void MergeTreeData::checkAlterPartitionIsPossible( void MergeTreeData::checkPartitionCanBeDropped(const ASTPtr & partition) { - const String partition_id = getPartitionIDFromQuery(partition, getContext()); DataPartsVector parts_to_remove; const auto * partition_ast = partition->as(); if (partition_ast && partition_ast->all) parts_to_remove = getDataPartsVector(); else + { + const String partition_id = getPartitionIDFromQuery(partition, getContext()); parts_to_remove = getDataPartsVectorInPartition(MergeTreeDataPartState::Active, partition_id); - + } UInt64 partition_size = 0; for (const auto & part : parts_to_remove) @@ -3828,6 +3836,9 @@ String MergeTreeData::getPartitionIDFromQuery(const ASTPtr & ast, ContextPtr loc { const auto & partition_ast = ast->as(); + if (partition_ast.all) + throw Exception("Only Support DETACH PARTITION ALL currently", ErrorCodes::SUPPORT_IS_DISABLED); + if (!partition_ast.value) { MergeTreePartInfo::validatePartitionID(partition_ast.id, format_version); @@ -3847,11 +3858,8 @@ String MergeTreeData::getPartitionIDFromQuery(const ASTPtr & ast, ContextPtr loc } /// Re-parse partition key fields using the information about expected field types. - auto metadata_snapshot = getInMemoryMetadataPtr(); const Block & key_sample_block = metadata_snapshot->getPartitionKey().sample_block; - if (partition_ast.all) - return "ALL"; size_t fields_count = key_sample_block.columns(); if (partition_ast.fields_count != fields_count) throw Exception(ErrorCodes::INVALID_PARTITION_VALUE, diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp index b1392f073ea..e7e4528dc83 100644 --- a/src/Storages/StorageMergeTree.cpp +++ b/src/Storages/StorageMergeTree.cpp @@ -1347,13 +1347,14 @@ void StorageMergeTree::dropPartition(const ASTPtr & partition, bool detach, Cont /// Asks to complete merges and does not allow them to start. /// This protects against "revival" of data for a removed partition after completion of merge. auto merge_blocker = stopMergesAndWait(); - String partition_id = getPartitionIDFromQuery(partition, local_context); const auto * partition_ast = partition->as(); if (partition_ast && partition_ast->all) parts_to_remove = getDataPartsVector(); else + { + String partition_id = getPartitionIDFromQuery(partition, local_context); parts_to_remove = getDataPartsVectorInPartition(MergeTreeDataPartState::Active, partition_id); - + } /// TODO should we throw an exception if parts_to_remove is empty? removePartsFromWorkingSet(parts_to_remove, true); } diff --git a/tests/queries/0_stateless/00753_alter_attach.sql b/tests/queries/0_stateless/00753_alter_attach.sql index 2910bcc222b..9fa4f92c2c1 100644 --- a/tests/queries/0_stateless/00753_alter_attach.sql +++ b/tests/queries/0_stateless/00753_alter_attach.sql @@ -66,6 +66,29 @@ select * from replicated_table_detach_all1 order by id; SYSTEM SYNC REPLICA replicated_table_detach_all2; select * from replicated_table_detach_all2 order by id; +ALTER TABLE replicated_table_detach_all1 FETCH PARTITION ALL FROM '/clickhouse/tables/test_00753_{database}/replicated_table_detach_all1'; -- { serverError 344 } + DROP TABLE replicated_table_detach_all1; DROP TABLE replicated_table_detach_all2; +DROP TABLE IF EXISTS partition_all; +DROP TABLE IF EXISTS partition_all2; + +CREATE TABLE partition_all (x UInt64, p UInt8, q UInt8) ENGINE = MergeTree ORDER BY tuple() PARTITION BY p; +INSERT INTO partition_all VALUES (4, 1, 2), (5, 1, 3), (3, 1, 4); + +CREATE TABLE partition_all2 (x UInt64, p UInt8, q UInt8) ENGINE = MergeTree ORDER BY tuple() PARTITION BY p; +INSERT INTO partition_all2 VALUES (4, 1, 2), (5, 1, 3), (3, 1, 4); + +-- test PARTITION ALL +ALTER TABLE partition_all2 REPLACE PARTITION ALL FROM partition_all; -- { serverError 344 } +ALTER TABLE partition_all MOVE PARTITION ALL TO TABLE partition_all2; -- { serverError 344 } +ALTER TABLE partition_all2 CLEAR INDEX p IN PARTITION ALL; -- { serverError 344 } +ALTER TABLE partition_all2 CLEAR COLUMN q IN PARTITION ALL; -- { serverError 344 } +ALTER TABLE partition_all2 UPDATE q = q + 1 IN PARTITION ALL where p = 1; -- { serverError 344 } +ALTER TABLE partition_all2 FREEZE PARTITION ALL; -- { serverError 344 } +CHECK TABLE partition_all2 PARTITION ALL; -- { serverError 344 } +OPTIMIZE TABLE partition_all2 PARTITION ALL; -- { serverError 344 } + +DROP TABLE partition_all; +DROP TABLE partition_all2;