diff --git a/src/Storages/MergeTree/registerStorageMergeTree.cpp b/src/Storages/MergeTree/registerStorageMergeTree.cpp index d48f85f3e4d..31687abf2f3 100644 --- a/src/Storages/MergeTree/registerStorageMergeTree.cpp +++ b/src/Storages/MergeTree/registerStorageMergeTree.cpp @@ -60,6 +60,27 @@ static Names extractColumnNames(const ASTPtr & node) } } +/** Is used to order Graphite::Retentions by age and precision descending. + * Throws exception if not both age and precision are less or greater then another. + */ +static bool compareRetentions(const Graphite::Retention & a, const Graphite::Retention & b) +{ + if (a.age > b.age && a.precision > b.precision) + { + return true; + } + else if (a.age < b.age && a.precision < b.precision) + { + return false; + } + String error_msg = "age and precision should only grow up: " + + std::to_string(a.age) + ":" + std::to_string(a.precision) + " vs " + + std::to_string(b.age) + ":" + std::to_string(b.precision); + throw Exception( + error_msg, + ErrorCodes::BAD_ARGUMENTS); +} + /** Read the settings for Graphite rollup from config. * Example * @@ -157,8 +178,7 @@ appendGraphitePattern(const Poco::Util::AbstractConfiguration & config, const St /// retention should be in descending order of age. if (pattern.type & pattern.TypeRetention) /// TypeRetention or TypeAll - std::sort(pattern.retentions.begin(), pattern.retentions.end(), - [] (const Graphite::Retention & a, const Graphite::Retention & b) { return a.age > b.age; }); + std::sort(pattern.retentions.begin(), pattern.retentions.end(), compareRetentions); patterns.emplace_back(pattern); } diff --git a/tests/integration/test_graphite_merge_tree/configs/graphite_rollup.xml b/tests/integration/test_graphite_merge_tree/configs/graphite_rollup.xml index 6d1907f3da7..f5206c8827a 100644 --- a/tests/integration/test_graphite_merge_tree/configs/graphite_rollup.xml +++ b/tests/integration/test_graphite_merge_tree/configs/graphite_rollup.xml @@ -94,4 +94,25 @@ + + metric + timestamp + value + updated + + avg + + 0 + 60 + + + 36000 + 600 + + + 72000 + 300 + + + diff --git a/tests/integration/test_graphite_merge_tree/test.py b/tests/integration/test_graphite_merge_tree/test.py index ee8a18f1ca5..7628211551d 100644 --- a/tests/integration/test_graphite_merge_tree/test.py +++ b/tests/integration/test_graphite_merge_tree/test.py @@ -3,6 +3,7 @@ import os.path as p import time import pytest +from helpers.client import QueryRuntimeException from helpers.cluster import ClickHouseCluster from helpers.test_tools import TSV @@ -442,3 +443,20 @@ SELECT * FROM test.graphite; ''') assert TSV(result) == TSV(expected) + + +def test_wrong_rollup_config(graphite_table): + with pytest.raises(QueryRuntimeException) as exc: + q(''' +CREATE TABLE test.graphite_not_created + (metric String, value Float64, timestamp UInt32, date Date, updated UInt32) + ENGINE = GraphiteMergeTree('graphite_rollup_wrong_age_precision') + PARTITION BY toYYYYMM(date) + ORDER BY (metric, timestamp) + SETTINGS index_granularity=1; + ''') + + # The order of retentions is not guaranteed + assert ("age and precision should only grow up: " in str(exc.value)) + assert ("36000:600" in str(exc.value)) + assert ("72000:300" in str(exc.value))