From 21599ffefc9e193ddf2c3ffc817b9d11f69928b5 Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 29 Jul 2021 16:11:05 +0300 Subject: [PATCH] Simple validation for partition id before drop partition --- src/Storages/MergeTree/MergeTreeData.cpp | 4 +++ src/Storages/MergeTree/MergeTreePartInfo.cpp | 34 +++++++++++++++++++ src/Storages/MergeTree/MergeTreePartInfo.h | 3 ++ .../01925_broken_partition_id_zookeeper.sql | 4 +-- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index a268468ea59..651f74622bb 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -3214,7 +3214,11 @@ String MergeTreeData::getPartitionIDFromQuery(const ASTPtr & ast, ContextPtr loc const auto & partition_ast = ast->as(); if (!partition_ast.value) + { + if (!MergeTreePartInfo::validatePartitionID(partition_ast.id, format_version)) + throw Exception("Invalid partition format: " + partition_ast.id, ErrorCodes::INVALID_PARTITION_VALUE); return partition_ast.id; + } if (format_version < MERGE_TREE_DATA_MIN_FORMAT_VERSION_WITH_CUSTOM_PARTITIONING) { diff --git a/src/Storages/MergeTree/MergeTreePartInfo.cpp b/src/Storages/MergeTree/MergeTreePartInfo.cpp index 94430de422e..24c18843935 100644 --- a/src/Storages/MergeTree/MergeTreePartInfo.cpp +++ b/src/Storages/MergeTree/MergeTreePartInfo.cpp @@ -21,6 +21,40 @@ MergeTreePartInfo MergeTreePartInfo::fromPartName(const String & part_name, Merg } +bool MergeTreePartInfo::validatePartitionID(const String & partition_id, MergeTreeDataFormatVersion format_version) +{ + if (partition_id.empty()) + return false; + + ReadBufferFromString in(partition_id); + + if (format_version < MERGE_TREE_DATA_MIN_FORMAT_VERSION_WITH_CUSTOM_PARTITIONING) + { + UInt32 min_yyyymmdd = 0; + UInt32 max_yyyymmdd = 0; + if (!tryReadIntText(min_yyyymmdd, in) + || !checkChar('_', in) + || !tryReadIntText(max_yyyymmdd, in) + || !checkChar('_', in)) + { + return false; + } + } + else + { + while (!in.eof()) + { + char c; + readChar(c, in); + + if (c == '_') + break; + } + } + + return in.eof(); +} + bool MergeTreePartInfo::tryParsePartName(const String & part_name, MergeTreePartInfo * part_info, MergeTreeDataFormatVersion format_version) { ReadBufferFromString in(part_name); diff --git a/src/Storages/MergeTree/MergeTreePartInfo.h b/src/Storages/MergeTree/MergeTreePartInfo.h index 8b77442bf8b..063f234e50f 100644 --- a/src/Storages/MergeTree/MergeTreePartInfo.h +++ b/src/Storages/MergeTree/MergeTreePartInfo.h @@ -86,6 +86,9 @@ struct MergeTreePartInfo return static_cast(max_block - min_block + 1); } + /// Simple sanity check for partition ID. Checking that it's not too long or too short, doesn't contain a lot of '_'. + static bool validatePartitionID(const String & partition_id, MergeTreeDataFormatVersion format_version); + static MergeTreePartInfo fromPartName(const String & part_name, MergeTreeDataFormatVersion format_version); // -V1071 static bool tryParsePartName(const String & part_name, MergeTreePartInfo * part_info, MergeTreeDataFormatVersion format_version); diff --git a/tests/queries/0_stateless/01925_broken_partition_id_zookeeper.sql b/tests/queries/0_stateless/01925_broken_partition_id_zookeeper.sql index bcd8bb4c87e..baf6c1fbf8f 100644 --- a/tests/queries/0_stateless/01925_broken_partition_id_zookeeper.sql +++ b/tests/queries/0_stateless/01925_broken_partition_id_zookeeper.sql @@ -9,8 +9,8 @@ ENGINE = ReplicatedMergeTree('/clickhouse/test_01925_{database}/rmt', 'r1') ORDER BY tuple() PARTITION BY date; -ALTER TABLE broken_partition DROP PARTITION ID '20210325_0_13241_6_12747'; +ALTER TABLE broken_partition DROP PARTITION ID '20210325_0_13241_6_12747'; --{serverError 248} -ALTER TABLE broken_partition DROP PARTITION ID '20210325_0_13241_6_12747'; +ALTER TABLE broken_partition DROP PARTITION ID '20210325_0_13241_6_12747'; --{serverError 248} DROP TABLE IF EXISTS broken_partition;