From 8de016d33aac87e22c2bb6941806fcf8577cce55 Mon Sep 17 00:00:00 2001 From: Amos Bird Date: Wed, 12 May 2021 23:16:55 +0800 Subject: [PATCH 1/2] Fix empty key projection query analysis --- src/Storages/MergeTree/MergeTreeData.cpp | 2 +- tests/queries/0_stateless/01710_projections.reference | 1 + tests/queries/0_stateless/01710_projections.sql | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 22fe540222e..802227ade98 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -4041,7 +4041,7 @@ bool MergeTreeData::getQueryProcessingStageWithAggregateProjection( auto required_columns = candidate.before_aggregation->foldActionsByProjection(keys, projection.sample_block_for_keys); // std::cerr << fmt::format("before_aggregation = \n{}", candidate.before_aggregation->dumpDAG()) << std::endl; // std::cerr << fmt::format("aggregate_required_columns = \n{}", fmt::join(required_columns, ", ")) << std::endl; - if (required_columns.empty()) + if (required_columns.empty() && !keys.empty()) continue; if (analysis_result.optimize_aggregation_in_order) diff --git a/tests/queries/0_stateless/01710_projections.reference b/tests/queries/0_stateless/01710_projections.reference index 9f30d82e23e..578f7523830 100644 --- a/tests/queries/0_stateless/01710_projections.reference +++ b/tests/queries/0_stateless/01710_projections.reference @@ -3,3 +3,4 @@ 2020-10-24 00:00:00 1.3619605237696326 0.16794469697335793 0.7637956767025532 0.8899329799574005 0.6227685185389797 0.30795997278638165 0.7637956767025532 2020-10-24 00:00:00 19 -1.9455094931672063 0.7759802460082872 0.6 0 2020-10-24 00:00:00 852 894 +999 diff --git a/tests/queries/0_stateless/01710_projections.sql b/tests/queries/0_stateless/01710_projections.sql index 49635b9a2c7..cac134ff216 100644 --- a/tests/queries/0_stateless/01710_projections.sql +++ b/tests/queries/0_stateless/01710_projections.sql @@ -39,3 +39,10 @@ select toStartOfMinute(datetime) dt_m, domain, sum(retry_count) / sum(duration), select toStartOfHour(toStartOfMinute(datetime)) dt_h, uniqHLL12(x_id), uniqHLL12(y_id) from projection_test group by dt_h order by dt_h; drop table if exists projection_test; + +drop table if exists projection_without_key; +create table projection_without_key (key UInt32, PROJECTION x (SELECT max(key))) engine MergeTree order by key; +insert into projection_without_key select number from numbers(1000); +set force_optimize_projection = 1, allow_experimental_projection_optimization = 1; +select max(key) from projection_without_key; +drop table projection_without_key; From a113acc40c570aad2ea9200ef0cd279881a43094 Mon Sep 17 00:00:00 2001 From: Amos Bird Date: Fri, 14 May 2021 22:26:09 +0800 Subject: [PATCH 2/2] Fix empty key --- src/Interpreters/Aggregator.cpp | 9 ++++++--- src/Processors/Transforms/AggregatingTransform.cpp | 7 ++++++- src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp | 6 +++++- tests/queries/0_stateless/01710_projections.reference | 1 + tests/queries/0_stateless/01710_projections.sql | 3 +++ 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Interpreters/Aggregator.cpp b/src/Interpreters/Aggregator.cpp index db047c0fc54..64b20a54955 100644 --- a/src/Interpreters/Aggregator.cpp +++ b/src/Interpreters/Aggregator.cpp @@ -1894,9 +1894,12 @@ void NO_INLINE Aggregator::mergeWithoutKeyStreamsImpl( res = place; } - /// Adding Values - for (size_t i = 0; i < params.aggregates_size; ++i) - aggregate_functions[i]->merge(res + offsets_of_aggregate_states[i], (*aggregate_columns[i])[0], result.aggregates_pool); + if (block.rows() > 0) + { + /// Adding Values + for (size_t i = 0; i < params.aggregates_size; ++i) + aggregate_functions[i]->merge(res + offsets_of_aggregate_states[i], (*aggregate_columns[i])[0], result.aggregates_pool); + } /// Early release memory. block.clear(); diff --git a/src/Processors/Transforms/AggregatingTransform.cpp b/src/Processors/Transforms/AggregatingTransform.cpp index 0724742e275..90e0e7cb066 100644 --- a/src/Processors/Transforms/AggregatingTransform.cpp +++ b/src/Processors/Transforms/AggregatingTransform.cpp @@ -546,7 +546,12 @@ void AggregatingTransform::initGenerate() /// If there was no data, and we aggregate without keys, and we must return single row with the result of empty aggregation. /// To do this, we pass a block with zero rows to aggregate. if (variants.empty() && params->params.keys_size == 0 && !params->params.empty_result_for_aggregation_by_empty_set) - params->aggregator.executeOnBlock(getInputs().front().getHeader(), variants, key_columns, aggregate_columns, no_more_keys); + { + if (params->only_merge) + params->aggregator.mergeBlock(getInputs().front().getHeader(), variants, no_more_keys); + else + params->aggregator.executeOnBlock(getInputs().front().getHeader(), variants, key_columns, aggregate_columns, no_more_keys); + } double elapsed_seconds = watch.elapsedSeconds(); size_t rows = variants.sizeWithoutOverflowRow(); diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 786536891cd..e22b9461b26 100644 --- a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -161,7 +161,11 @@ QueryPlanPtr MergeTreeDataSelectExecutor::read( query_info.merge_tree_data_select_cache.get()); } - LOG_DEBUG(log, "Choose projection {}", query_info.projection->desc->name); + LOG_DEBUG( + log, + "Choose {} projection {}", + ProjectionDescription::typeToString(query_info.projection->desc->type), + query_info.projection->desc->name); if (query_info.projection->merge_tree_data_select_base_cache->sum_marks + query_info.projection->merge_tree_data_select_projection_cache->sum_marks diff --git a/tests/queries/0_stateless/01710_projections.reference b/tests/queries/0_stateless/01710_projections.reference index 578f7523830..1e4f659c639 100644 --- a/tests/queries/0_stateless/01710_projections.reference +++ b/tests/queries/0_stateless/01710_projections.reference @@ -3,4 +3,5 @@ 2020-10-24 00:00:00 1.3619605237696326 0.16794469697335793 0.7637956767025532 0.8899329799574005 0.6227685185389797 0.30795997278638165 0.7637956767025532 2020-10-24 00:00:00 19 -1.9455094931672063 0.7759802460082872 0.6 0 2020-10-24 00:00:00 852 894 +2 -1 999 diff --git a/tests/queries/0_stateless/01710_projections.sql b/tests/queries/0_stateless/01710_projections.sql index cac134ff216..c929bfaa273 100644 --- a/tests/queries/0_stateless/01710_projections.sql +++ b/tests/queries/0_stateless/01710_projections.sql @@ -38,6 +38,9 @@ select toStartOfMinute(datetime) dt_m, domain, sum(retry_count) / sum(duration), select toStartOfHour(toStartOfMinute(datetime)) dt_h, uniqHLL12(x_id), uniqHLL12(y_id) from projection_test group by dt_h order by dt_h; +-- found by fuzzer +SELECT 2, -1 FROM projection_test PREWHERE domain_alias = 1. WHERE domain = NULL GROUP BY -9223372036854775808 ORDER BY countIf(first_time = 0) / count(-2147483649) DESC NULLS LAST, 1048576 DESC NULLS LAST; + drop table if exists projection_test; drop table if exists projection_without_key;