From 5e2bfd5ba1c3331401b5f75a8bbdb25a948c9cb4 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Fri, 6 Aug 2021 21:03:38 +0300 Subject: [PATCH] fix partition id validation --- src/Storages/MergeTree/MergeTreeData.cpp | 8 ++--- src/Storages/MergeTree/MergeTreePartInfo.cpp | 32 ++++++------------- src/Storages/MergeTree/MergeTreePartInfo.h | 2 +- .../01925_broken_partition_id_zookeeper.sql | 10 ++++++ 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 60ff3d094b7..4730bf9f47c 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -3213,8 +3213,7 @@ String MergeTreeData::getPartitionIDFromQuery(const ASTPtr & ast, ContextPtr loc if (!partition_ast.value) { - if (!MergeTreePartInfo::validatePartitionID(partition_ast.id, format_version)) - throw Exception("Invalid partition format: " + partition_ast.id, ErrorCodes::INVALID_PARTITION_VALUE); + MergeTreePartInfo::validatePartitionID(partition_ast.id, format_version); return partition_ast.id; } @@ -3225,10 +3224,7 @@ String MergeTreeData::getPartitionIDFromQuery(const ASTPtr & ast, ContextPtr loc if (partition_lit && partition_lit->value.getType() == Field::Types::String) { String partition_id = partition_lit->value.get(); - if (partition_id.size() != 6 || !std::all_of(partition_id.begin(), partition_id.end(), isNumericASCII)) - throw Exception( - "Invalid partition format: " + partition_id + ". Partition should consist of 6 digits: YYYYMM", - ErrorCodes::INVALID_PARTITION_VALUE); + MergeTreePartInfo::validatePartitionID(partition_id, format_version); return partition_id; } } diff --git a/src/Storages/MergeTree/MergeTreePartInfo.cpp b/src/Storages/MergeTree/MergeTreePartInfo.cpp index ccb26a0999e..6a98e666c34 100644 --- a/src/Storages/MergeTree/MergeTreePartInfo.cpp +++ b/src/Storages/MergeTree/MergeTreePartInfo.cpp @@ -9,6 +9,7 @@ namespace DB namespace ErrorCodes { extern const int BAD_DATA_PART_NAME; + extern const int INVALID_PARTITION_VALUE; } @@ -21,38 +22,25 @@ MergeTreePartInfo MergeTreePartInfo::fromPartName(const String & part_name, Merg } -bool MergeTreePartInfo::validatePartitionID(const String & partition_id, MergeTreeDataFormatVersion format_version) +void MergeTreePartInfo::validatePartitionID(const String & partition_id, MergeTreeDataFormatVersion format_version) { if (partition_id.empty()) - return false; - - ReadBufferFromString in(partition_id); + throw Exception(ErrorCodes::INVALID_PARTITION_VALUE, "Partition id is empty"); 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; - } + if (partition_id.size() != 6 || !std::all_of(partition_id.begin(), partition_id.end(), isNumericASCII)) + throw Exception(ErrorCodes::INVALID_PARTITION_VALUE, + "Invalid partition format: {}. Partition should consist of 6 digits: YYYYMM", + partition_id); } else { - while (!in.eof()) - { - char c; - readChar(c, in); - - if (c == '_') - break; - } + auto is_valid_char = [](char c) { return c == '-' || isAlphaNumericASCII(c); }; + if (!std::all_of(partition_id.begin(), partition_id.end(), is_valid_char)) + throw Exception(ErrorCodes::INVALID_PARTITION_VALUE, "Invalid partition format: {}", partition_id); } - return in.eof(); } bool MergeTreePartInfo::tryParsePartName(const String & part_name, MergeTreePartInfo * part_info, MergeTreeDataFormatVersion format_version) diff --git a/src/Storages/MergeTree/MergeTreePartInfo.h b/src/Storages/MergeTree/MergeTreePartInfo.h index 87f96ed5038..be856c1f157 100644 --- a/src/Storages/MergeTree/MergeTreePartInfo.h +++ b/src/Storages/MergeTree/MergeTreePartInfo.h @@ -88,7 +88,7 @@ struct MergeTreePartInfo } /// 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 void validatePartitionID(const String & partition_id, MergeTreeDataFormatVersion format_version); static MergeTreePartInfo fromPartName(const String & part_name, MergeTreeDataFormatVersion format_version); // -V1071 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 baf6c1fbf8f..07e490d0ce0 100644 --- a/tests/queries/0_stateless/01925_broken_partition_id_zookeeper.sql +++ b/tests/queries/0_stateless/01925_broken_partition_id_zookeeper.sql @@ -14,3 +14,13 @@ ALTER TABLE broken_partition DROP PARTITION ID '20210325_0_13241_6_12747'; --{se ALTER TABLE broken_partition DROP PARTITION ID '20210325_0_13241_6_12747'; --{serverError 248} DROP TABLE IF EXISTS broken_partition; + +DROP TABLE IF EXISTS old_partition_key; + +CREATE TABLE old_partition_key (sd Date, dh UInt64, ak UInt32, ed Date) ENGINE=MergeTree(sd, dh, (ak, ed, dh), 8192); + +ALTER TABLE old_partition_key DROP PARTITION ID '20210325_0_13241_6_12747'; --{serverError 248} + +ALTER TABLE old_partition_key DROP PARTITION ID '202103'; + +DROP TABLE old_partition_key;