From 7fa72f5cdfd61de0c4b79e4275f142c452ce60e6 Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 18 Aug 2023 12:47:26 +0000 Subject: [PATCH 01/10] Optimize group by constant keys --- src/Common/BoolArgsToTemplateArgsDispatcher.h | 40 +++++ src/Core/Settings.h | 1 + src/Core/SettingsChangesHistory.h | 3 +- src/Interpreters/Aggregator.cpp | 140 ++++++++++++------ src/Interpreters/Aggregator.h | 8 +- src/Interpreters/InterpreterSelectQuery.cpp | 1 + src/Planner/Planner.cpp | 1 + src/Processors/QueryPlan/AggregatingStep.cpp | 1 + .../TTL/TTLAggregationAlgorithm.cpp | 3 +- .../MergeTree/MergeTreeDataSelectExecutor.cpp | 3 +- .../02845_group_by_constant_keys.reference | 4 + .../02845_group_by_constant_keys.sql | 5 + 12 files changed, 160 insertions(+), 50 deletions(-) create mode 100644 src/Common/BoolArgsToTemplateArgsDispatcher.h create mode 100644 tests/queries/0_stateless/02845_group_by_constant_keys.reference create mode 100644 tests/queries/0_stateless/02845_group_by_constant_keys.sql diff --git a/src/Common/BoolArgsToTemplateArgsDispatcher.h b/src/Common/BoolArgsToTemplateArgsDispatcher.h new file mode 100644 index 000000000000..086b1e4b0f43 --- /dev/null +++ b/src/Common/BoolArgsToTemplateArgsDispatcher.h @@ -0,0 +1,40 @@ +#pragma once +#include + +namespace DB +{ + + +/// Special struct that helps to convert bool variables to function template bool arguments. +/// It can be used to avoid multiple nested if/else on bool arguments. How to use it: +/// Imagine you have template function +/// template return_type foo(...); +/// and bool variables b1, b2, ... bn. To pass these variables as template for foo you can do the following: +/// +/// auto call_foo = []() +/// { +/// return foo(...); +/// } +/// +/// BoolArgsToTemplateArgsDispatcher::call(call_foo, b1, b2, ..., bn); + +template +struct BoolArgsToTemplateArgsDispatcher +{ + template + static auto call(Func f, Args1&&... args) + { + return f.template operator()(std::forward(args)...); + } + + template + static auto call(Func f, bool b, Args1&&... ar1) + { + if (b) + return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); + else + return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); + } +}; + +} diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 6c3d339b4bee..418ae08ac7fd 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -650,6 +650,7 @@ class IColumn; M(SetOperationMode, except_default_mode, SetOperationMode::ALL, "Set default mode in EXCEPT query. Possible values: empty string, 'ALL', 'DISTINCT'. If empty, query without mode will throw exception.", 0) \ M(Bool, optimize_aggregators_of_group_by_keys, true, "Eliminates min/max/any/anyLast aggregators of GROUP BY keys in SELECT section", 0) \ M(Bool, optimize_group_by_function_keys, true, "Eliminates functions of other keys in GROUP BY section", 0) \ + M(Bool, optimize_group_by_constant_keys, true, "Optimize GROUP BY when all keys in block are constant", 0) \ M(Bool, legacy_column_name_of_tuple_literal, false, "List all names of element of large tuple literals in their column names instead of hash. This settings exists only for compatibility reasons. It makes sense to set to 'true', while doing rolling update of cluster from version lower than 21.7 to higher.", 0) \ \ M(Bool, query_plan_enable_optimizations, true, "Apply optimizations to query plan", 0) \ diff --git a/src/Core/SettingsChangesHistory.h b/src/Core/SettingsChangesHistory.h index dcb67165addf..eaa932a35208 100644 --- a/src/Core/SettingsChangesHistory.h +++ b/src/Core/SettingsChangesHistory.h @@ -80,7 +80,8 @@ namespace SettingsChangesHistory /// It's used to implement `compatibility` setting (see https://github.com/ClickHouse/ClickHouse/issues/35972) static std::map settings_changes_history = { - {"23.8", {{"rewrite_count_distinct_if_with_count_distinct_implementation", false, true, "Rewrite countDistinctIf with count_distinct_implementation configuration"}}}, + {"23.8", {{"rewrite_count_distinct_if_with_count_distinct_implementation", false, true, "Rewrite countDistinctIf with count_distinct_implementation configuration"}, + {"optimize_group_by_constant_keys", false, true, "Optimize group by constant keys by default"}}}, {"23.7", {{"function_sleep_max_microseconds_per_block", 0, 3000000, "In previous versions, the maximum sleep time of 3 seconds was applied only for `sleep`, but not for `sleepEachRow` function. In the new version, we introduce this setting. If you set compatibility with the previous versions, we will disable the limit altogether."}}}, {"23.6", {{"http_send_timeout", 180, 30, "3 minutes seems crazy long. Note that this is timeout for a single network write call, not for the whole upload operation."}, {"http_receive_timeout", 180, 30, "See http_send_timeout."}}}, diff --git a/src/Interpreters/Aggregator.cpp b/src/Interpreters/Aggregator.cpp index 91cd574708a1..72ea2c8ec425 100644 --- a/src/Interpreters/Aggregator.cpp +++ b/src/Interpreters/Aggregator.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include @@ -1036,11 +1037,12 @@ void Aggregator::executeImpl( ColumnRawPtrs & key_columns, AggregateFunctionInstruction * aggregate_instructions, bool no_more_keys, + bool all_keys_are_const, AggregateDataPtr overflow_row) const { #define M(NAME, IS_TWO_LEVEL) \ else if (result.type == AggregatedDataVariants::Type::NAME) \ - executeImpl(*result.NAME, result.aggregates_pool, row_begin, row_end, key_columns, aggregate_instructions, no_more_keys, overflow_row); + executeImpl(*result.NAME, result.aggregates_pool, row_begin, row_end, key_columns, aggregate_instructions, no_more_keys, all_keys_are_const, overflow_row); if (false) {} // NOLINT APPLY_FOR_AGGREGATED_VARIANTS(M) @@ -1060,44 +1062,35 @@ void NO_INLINE Aggregator::executeImpl( ColumnRawPtrs & key_columns, AggregateFunctionInstruction * aggregate_instructions, bool no_more_keys, + bool all_keys_are_const, AggregateDataPtr overflow_row) const { typename Method::State state(key_columns, key_sizes, aggregation_state_cache); + auto call_execute_impl_batch = [&]() + { + executeImplBatch(method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); + }; + + bool use_compiled_functions = false; +#if USE_EMBEDDED_COMPILER + use_compiled_functions = compiled_aggregate_functions_holder && !hasSparseArguments(aggregate_instructions); +#endif + if (!no_more_keys) { /// Prefetching doesn't make sense for small hash tables, because they fit in caches entirely. const bool prefetch = Method::State::has_cheap_key_calculation && params.enable_prefetch && (method.data.getBufferSizeInBytes() > min_bytes_for_prefetch); - -#if USE_EMBEDDED_COMPILER - if (compiled_aggregate_functions_holder && !hasSparseArguments(aggregate_instructions)) - { - if (prefetch) - executeImplBatch( - method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); - else - executeImplBatch( - method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); - } - else -#endif - { - if (prefetch) - executeImplBatch( - method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); - else - executeImplBatch( - method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); - } + BoolArgsToTemplateArgsDispatcher::call(call_execute_impl_batch, no_more_keys, use_compiled_functions, prefetch, all_keys_are_const); } else { - executeImplBatch(method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); + BoolArgsToTemplateArgsDispatcher::call(call_execute_impl_batch, no_more_keys, false, false, all_keys_are_const); } } -template +template void NO_INLINE Aggregator::executeImplBatch( Method & method, typename Method::State & state, @@ -1121,27 +1114,34 @@ void NO_INLINE Aggregator::executeImplBatch( /// For all rows. AggregateDataPtr place = aggregates_pool->alloc(0); - for (size_t i = row_begin; i < row_end; ++i) + if (all_keys_are_const) { - if constexpr (prefetch && HasPrefetchMemberFunc) + state.emplaceKey(method.data, 0, *aggregates_pool).setMapped(place); + } + else + { + for (size_t i = row_begin; i < row_end; ++i) { - if (i == row_begin + prefetching.iterationsToMeasure()) - prefetch_look_ahead = prefetching.calcPrefetchLookAhead(); - - if (i + prefetch_look_ahead < row_end) + if constexpr (prefetch && HasPrefetchMemberFunc) { - auto && key_holder = state.getKeyHolder(i + prefetch_look_ahead, *aggregates_pool); - method.data.prefetch(std::move(key_holder)); + if (i == row_begin + prefetching.iterationsToMeasure()) + prefetch_look_ahead = prefetching.calcPrefetchLookAhead(); + + if (i + prefetch_look_ahead < row_end) + { + auto && key_holder = state.getKeyHolder(i + prefetch_look_ahead, *aggregates_pool); + method.data.prefetch(std::move(key_holder)); + } } - } - state.emplaceKey(method.data, i, *aggregates_pool).setMapped(place); + state.emplaceKey(method.data, i, *aggregates_pool).setMapped(place); + } } return; } /// Optimization for special case when aggregating by 8bit key. - if constexpr (!no_more_keys && std::is_same_v) + if constexpr (!no_more_keys && !all_keys_are_const && std::is_same_v) { /// We use another method if there are aggregate functions with -Array combinator. bool has_arrays = false; @@ -1183,7 +1183,20 @@ void NO_INLINE Aggregator::executeImplBatch( std::unique_ptr places(new AggregateDataPtr[row_end]); /// For all rows. - for (size_t i = row_begin; i < row_end; ++i) + size_t start, end; + /// If all keys are const, key columns contain only 1 row. + if constexpr (all_keys_are_const) + { + start = 0; + end = 1; + } + else + { + start = row_begin; + end = row_end; + } + + for (size_t i = start; i < end; ++i) { AggregateDataPtr aggregate_data = nullptr; @@ -1279,8 +1292,16 @@ void NO_INLINE Aggregator::executeImplBatch( columns_data.emplace_back(getColumnData(inst->batch_arguments[argument_index])); } - auto add_into_aggregate_states_function = compiled_aggregate_functions_holder->compiled_aggregate_functions.add_into_aggregate_states_function; - add_into_aggregate_states_function(row_begin, row_end, columns_data.data(), places.get()); + if constexpr (all_keys_are_const) + { + auto add_into_aggregate_states_function_single_place = compiled_aggregate_functions_holder->compiled_aggregate_functions.add_into_aggregate_states_function_single_place; + add_into_aggregate_states_function_single_place(row_begin, row_end, columns_data.data(), places[0]); + } + else + { + auto add_into_aggregate_states_function = compiled_aggregate_functions_holder->compiled_aggregate_functions.add_into_aggregate_states_function; + add_into_aggregate_states_function(row_begin, row_end, columns_data.data(), places.get()); + } } #endif @@ -1295,12 +1316,24 @@ void NO_INLINE Aggregator::executeImplBatch( AggregateFunctionInstruction * inst = aggregate_instructions + i; - if (inst->offsets) - inst->batch_that->addBatchArray(row_begin, row_end, places.get(), inst->state_offset, inst->batch_arguments, inst->offsets, aggregates_pool); - else if (inst->has_sparse_arguments) - inst->batch_that->addBatchSparse(row_begin, row_end, places.get(), inst->state_offset, inst->batch_arguments, aggregates_pool); + if constexpr (all_keys_are_const) + { + if (inst->offsets) + inst->batch_that->addBatchSinglePlace(inst->offsets[static_cast(row_begin) - 1], inst->offsets[row_end - 1], places[0] + inst->state_offset, inst->batch_arguments, aggregates_pool); + else if (inst->has_sparse_arguments) + inst->batch_that->addBatchSparseSinglePlace(row_begin, row_end, places[0] + inst->state_offset, inst->batch_arguments, aggregates_pool); + else + inst->batch_that->addBatchSinglePlace(row_begin, row_end, places[0] + inst->state_offset, inst->batch_arguments, aggregates_pool); + } else - inst->batch_that->addBatch(row_begin, row_end, places.get(), inst->state_offset, inst->batch_arguments, aggregates_pool); + { + if (inst->offsets) + inst->batch_that->addBatchArray(row_begin, row_end, places.get(), inst->state_offset, inst->batch_arguments, inst->offsets, aggregates_pool); + else if (inst->has_sparse_arguments) + inst->batch_that->addBatchSparse(row_begin, row_end, places.get(), inst->state_offset, inst->batch_arguments, aggregates_pool); + else + inst->batch_that->addBatch(row_begin, row_end, places.get(), inst->state_offset, inst->batch_arguments, aggregates_pool); + } } } @@ -1540,12 +1573,27 @@ bool Aggregator::executeOnBlock(Columns columns, * To make them work anyway, we materialize them. */ Columns materialized_columns; + bool all_keys_are_const = false; + if (params.optimize_group_by_constant_keys) + { + all_keys_are_const = true; + for (size_t i = 0; i < params.keys_size; ++i) + all_keys_are_const &= isColumnConst(*columns.at(keys_positions[i])); + } /// Remember the columns we will work with for (size_t i = 0; i < params.keys_size; ++i) { - materialized_columns.push_back(recursiveRemoveSparse(columns.at(keys_positions[i]))->convertToFullColumnIfConst()); - key_columns[i] = materialized_columns.back().get(); + if (all_keys_are_const) + { + key_columns[i] = assert_cast(*columns.at(keys_positions[i])).getDataColumnPtr().get(); + } + else + { + materialized_columns.push_back(recursiveRemoveSparse(columns.at(keys_positions[i]))->convertToFullColumnIfConst()); + key_columns[i] = materialized_columns.back().get(); + } + if (!result.isLowCardinality()) { @@ -1590,7 +1638,7 @@ bool Aggregator::executeOnBlock(Columns columns, { /// This is where data is written that does not fit in `max_rows_to_group_by` with `group_by_overflow_mode = any`. AggregateDataPtr overflow_row_ptr = params.overflow_row ? result.without_key : nullptr; - executeImpl(result, row_begin, row_end, key_columns, aggregate_functions_instructions.data(), no_more_keys, overflow_row_ptr); + executeImpl(result, row_begin, row_end, key_columns, aggregate_functions_instructions.data(), no_more_keys, all_keys_are_const, overflow_row_ptr); } size_t result_size = result.sizeWithoutOverflowRow(); diff --git a/src/Interpreters/Aggregator.h b/src/Interpreters/Aggregator.h index 4f2c86606c52..dc0391c4289a 100644 --- a/src/Interpreters/Aggregator.h +++ b/src/Interpreters/Aggregator.h @@ -1020,6 +1020,8 @@ class Aggregator final bool enable_prefetch; + bool optimize_group_by_constant_keys; + struct StatsCollectingParams { StatsCollectingParams(); @@ -1057,6 +1059,7 @@ class Aggregator final size_t max_block_size_, bool enable_prefetch_, bool only_merge_, // true for projections + bool optimize_group_by_constant_keys_, const StatsCollectingParams & stats_collecting_params_ = {}) : keys(keys_) , aggregates(aggregates_) @@ -1077,6 +1080,7 @@ class Aggregator final , max_block_size(max_block_size_) , only_merge(only_merge_) , enable_prefetch(enable_prefetch_) + , optimize_group_by_constant_keys(optimize_group_by_constant_keys_) , stats_collecting_params(stats_collecting_params_) { } @@ -1276,6 +1280,7 @@ class Aggregator final ColumnRawPtrs & key_columns, AggregateFunctionInstruction * aggregate_instructions, bool no_more_keys = false, + bool all_keys_are_const = false, AggregateDataPtr overflow_row = nullptr) const; /// Process one data block, aggregate the data into a hash table. @@ -1288,10 +1293,11 @@ class Aggregator final ColumnRawPtrs & key_columns, AggregateFunctionInstruction * aggregate_instructions, bool no_more_keys, + bool all_keys_are_const, AggregateDataPtr overflow_row) const; /// Specialization for a particular value no_more_keys. - template + template void executeImplBatch( Method & method, typename Method::State & state, diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp index 20fca1b1e765..8839bd8f7e25 100644 --- a/src/Interpreters/InterpreterSelectQuery.cpp +++ b/src/Interpreters/InterpreterSelectQuery.cpp @@ -2579,6 +2579,7 @@ static Aggregator::Params getAggregatorParams( settings.max_block_size, settings.enable_software_prefetch_in_aggregation, /* only_merge */ false, + settings.optimize_group_by_constant_keys, stats_collecting_params }; } diff --git a/src/Planner/Planner.cpp b/src/Planner/Planner.cpp index 9f6c22f90f39..a75031898c42 100644 --- a/src/Planner/Planner.cpp +++ b/src/Planner/Planner.cpp @@ -290,6 +290,7 @@ Aggregator::Params getAggregatorParams(const PlannerContextPtr & planner_context settings.max_block_size, settings.enable_software_prefetch_in_aggregation, /* only_merge */ false, + settings.optimize_group_by_constant_keys, stats_collecting_params); return aggregator_params; diff --git a/src/Processors/QueryPlan/AggregatingStep.cpp b/src/Processors/QueryPlan/AggregatingStep.cpp index eebbfc043047..c68a070f8166 100644 --- a/src/Processors/QueryPlan/AggregatingStep.cpp +++ b/src/Processors/QueryPlan/AggregatingStep.cpp @@ -230,6 +230,7 @@ void AggregatingStep::transformPipeline(QueryPipelineBuilder & pipeline, const B transform_params->params.max_block_size, transform_params->params.enable_prefetch, /* only_merge */ false, + transform_params->params.optimize_group_by_constant_keys, transform_params->params.stats_collecting_params}; auto transform_params_for_set = std::make_shared(src_header, std::move(params_for_set), final); diff --git a/src/Processors/TTL/TTLAggregationAlgorithm.cpp b/src/Processors/TTL/TTLAggregationAlgorithm.cpp index cdd6e3e917f8..fa3436ec55d8 100644 --- a/src/Processors/TTL/TTLAggregationAlgorithm.cpp +++ b/src/Processors/TTL/TTLAggregationAlgorithm.cpp @@ -41,7 +41,8 @@ TTLAggregationAlgorithm::TTLAggregationAlgorithm( settings.min_count_to_compile_aggregate_expression, settings.max_block_size, settings.enable_software_prefetch_in_aggregation, - false /* only_merge */); + false /* only_merge */, + settings.optimize_group_by_constant_keys); aggregator = std::make_unique(header, params); diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index ee515106591a..8f9d54d942d3 100644 --- a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -328,7 +328,8 @@ QueryPlanPtr MergeTreeDataSelectExecutor::read( settings.min_count_to_compile_aggregate_expression, settings.max_block_size, settings.enable_software_prefetch_in_aggregation, - only_merge); + only_merge, + settings.optimize_group_by_constant_keys); return std::make_pair(params, only_merge); }; diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.reference b/tests/queries/0_stateless/02845_group_by_constant_keys.reference new file mode 100644 index 000000000000..60e40ea54a7b --- /dev/null +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.reference @@ -0,0 +1,4 @@ +10000000 1 2 3 +10000000 1 2 3 +10000000 1 2 3 +10000000 1 2 3 diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.sql b/tests/queries/0_stateless/02845_group_by_constant_keys.sql new file mode 100644 index 000000000000..0223cf1df605 --- /dev/null +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.sql @@ -0,0 +1,5 @@ +SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 0; +SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions = 1; +SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 1; + From 1f5e209897b4d45dbaa6c9e2588bcc7a536702a4 Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 18 Aug 2023 12:50:04 +0000 Subject: [PATCH 02/10] Better comment --- src/Common/BoolArgsToTemplateArgsDispatcher.h | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Common/BoolArgsToTemplateArgsDispatcher.h b/src/Common/BoolArgsToTemplateArgsDispatcher.h index 086b1e4b0f43..469bc34cfbf2 100644 --- a/src/Common/BoolArgsToTemplateArgsDispatcher.h +++ b/src/Common/BoolArgsToTemplateArgsDispatcher.h @@ -4,36 +4,35 @@ namespace DB { - /// Special struct that helps to convert bool variables to function template bool arguments. /// It can be used to avoid multiple nested if/else on bool arguments. How to use it: /// Imagine you have template function /// template return_type foo(...); -/// and bool variables b1, b2, ... bn. To pass these variables as template for foo you can do the following: +/// and bool variables b1, b2, ..., bool bn. To pass these variables as template for foo you can do the following: /// -/// auto call_foo = []() +/// auto call_foo = []() /// { /// return foo(...); /// } /// /// BoolArgsToTemplateArgsDispatcher::call(call_foo, b1, b2, ..., bn); -template +template struct BoolArgsToTemplateArgsDispatcher { template - static auto call(Func f, Args1&&... args) + static auto call(Functor f, Args1&&... args) { return f.template operator()(std::forward(args)...); } template - static auto call(Func f, bool b, Args1&&... ar1) + static auto call(Functor f, bool b, Args1&&... ar1) { if (b) - return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); + return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); else - return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); + return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); } }; From aac513185952a8d12f6e9e0247623a63d3c39ee4 Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 18 Aug 2023 13:21:16 +0000 Subject: [PATCH 03/10] Fix for no_more_keys --- src/Common/BoolArgsToTemplateArgsDispatcher.h | 4 ++-- src/Interpreters/Aggregator.cpp | 12 ++++++++++-- src/Storages/StorageFile.cpp | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Common/BoolArgsToTemplateArgsDispatcher.h b/src/Common/BoolArgsToTemplateArgsDispatcher.h index 469bc34cfbf2..3e72db49cd26 100644 --- a/src/Common/BoolArgsToTemplateArgsDispatcher.h +++ b/src/Common/BoolArgsToTemplateArgsDispatcher.h @@ -7,8 +7,8 @@ namespace DB /// Special struct that helps to convert bool variables to function template bool arguments. /// It can be used to avoid multiple nested if/else on bool arguments. How to use it: /// Imagine you have template function -/// template return_type foo(...); -/// and bool variables b1, b2, ..., bool bn. To pass these variables as template for foo you can do the following: +/// template return_type foo(...); +/// and bool variables b1, b2, ..., bn. To pass these variables as template for foo you can do the following: /// /// auto call_foo = []() /// { diff --git a/src/Interpreters/Aggregator.cpp b/src/Interpreters/Aggregator.cpp index 72ea2c8ec425..6392428c5e6d 100644 --- a/src/Interpreters/Aggregator.cpp +++ b/src/Interpreters/Aggregator.cpp @@ -1180,7 +1180,7 @@ void NO_INLINE Aggregator::executeImplBatch( /// - this affects only optimize_aggregation_in_order, /// - this is just a pointer, so it should not be significant, /// - and plus this will require other changes in the interface. - std::unique_ptr places(new AggregateDataPtr[row_end]); + std::unique_ptr places(new AggregateDataPtr[all_keys_are_const ? 1 : row_end]); /// For all rows. size_t start, end; @@ -1202,7 +1202,7 @@ void NO_INLINE Aggregator::executeImplBatch( if constexpr (!no_more_keys) { - if constexpr (prefetch && HasPrefetchMemberFunc) + if constexpr (prefetch && !all_keys_are_const && HasPrefetchMemberFunc) { if (i == row_begin + prefetching.iterationsToMeasure()) prefetch_look_ahead = prefetching.calcPrefetchLookAhead(); @@ -1267,9 +1267,17 @@ void NO_INLINE Aggregator::executeImplBatch( /// Add only if the key already exists. auto find_result = state.findKey(method.data, i, *aggregates_pool); if (find_result.isFound()) + { aggregate_data = find_result.getMapped(); + } else + { + /// If all keys are constant and this is new key + /// we don't need to do anything and just skip the whole block. + if constexpr (all_keys_are_const) + return; aggregate_data = overflow_row; + } } places[i] = aggregate_data; diff --git a/src/Storages/StorageFile.cpp b/src/Storages/StorageFile.cpp index e3908c75a588..57e04f547d4b 100644 --- a/src/Storages/StorageFile.cpp +++ b/src/Storages/StorageFile.cpp @@ -965,13 +965,13 @@ class StorageFileSource : public ISource { if (virtual_column.name == "_path") { - chunk.addColumn(virtual_column.type->createColumnConst(num_rows, current_path)->convertToFullColumnIfConst()); + chunk.addColumn(virtual_column.type->createColumnConst(num_rows, current_path)); } else if (virtual_column.name == "_file") { size_t last_slash_pos = current_path.find_last_of('/'); auto file_name = current_path.substr(last_slash_pos + 1); - chunk.addColumn(virtual_column.type->createColumnConst(num_rows, file_name)->convertToFullColumnIfConst()); + chunk.addColumn(virtual_column.type->createColumnConst(num_rows, file_name)); } } From 0da8fe2793b88dbc3fd7465644e154b872a75000 Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 18 Aug 2023 13:22:36 +0000 Subject: [PATCH 04/10] Add missing constexpr --- src/Interpreters/Aggregator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Interpreters/Aggregator.cpp b/src/Interpreters/Aggregator.cpp index 6392428c5e6d..0d4ebfd1081b 100644 --- a/src/Interpreters/Aggregator.cpp +++ b/src/Interpreters/Aggregator.cpp @@ -1114,7 +1114,7 @@ void NO_INLINE Aggregator::executeImplBatch( /// For all rows. AggregateDataPtr place = aggregates_pool->alloc(0); - if (all_keys_are_const) + if constexpr (all_keys_are_const) { state.emplaceKey(method.data, 0, *aggregates_pool).setMapped(place); } From 2d4167c61206e256a36e647c878d3f98f40a2724 Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 18 Aug 2023 14:50:07 +0000 Subject: [PATCH 05/10] FRemove unneded changes --- src/Storages/StorageFile.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Storages/StorageFile.cpp b/src/Storages/StorageFile.cpp index 57e04f547d4b..e3908c75a588 100644 --- a/src/Storages/StorageFile.cpp +++ b/src/Storages/StorageFile.cpp @@ -965,13 +965,13 @@ class StorageFileSource : public ISource { if (virtual_column.name == "_path") { - chunk.addColumn(virtual_column.type->createColumnConst(num_rows, current_path)); + chunk.addColumn(virtual_column.type->createColumnConst(num_rows, current_path)->convertToFullColumnIfConst()); } else if (virtual_column.name == "_file") { size_t last_slash_pos = current_path.find_last_of('/'); auto file_name = current_path.substr(last_slash_pos + 1); - chunk.addColumn(virtual_column.type->createColumnConst(num_rows, file_name)); + chunk.addColumn(virtual_column.type->createColumnConst(num_rows, file_name)->convertToFullColumnIfConst()); } } From 59ca2374b583efdeaf81e30417b756213850fae8 Mon Sep 17 00:00:00 2001 From: avogar Date: Wed, 23 Aug 2023 19:39:45 +0000 Subject: [PATCH 06/10] Better --- src/Common/BoolArgsToTemplateArgsDispatcher.h | 39 -------------- src/Interpreters/Aggregator.cpp | 54 +++++++++++-------- src/Interpreters/Aggregator.h | 3 +- .../02845_group_by_constant_keys.reference | 36 +++++++++++++ .../02845_group_by_constant_keys.sql | 32 +++++++++-- 5 files changed, 98 insertions(+), 66 deletions(-) delete mode 100644 src/Common/BoolArgsToTemplateArgsDispatcher.h diff --git a/src/Common/BoolArgsToTemplateArgsDispatcher.h b/src/Common/BoolArgsToTemplateArgsDispatcher.h deleted file mode 100644 index 3e72db49cd26..000000000000 --- a/src/Common/BoolArgsToTemplateArgsDispatcher.h +++ /dev/null @@ -1,39 +0,0 @@ -#pragma once -#include - -namespace DB -{ - -/// Special struct that helps to convert bool variables to function template bool arguments. -/// It can be used to avoid multiple nested if/else on bool arguments. How to use it: -/// Imagine you have template function -/// template return_type foo(...); -/// and bool variables b1, b2, ..., bn. To pass these variables as template for foo you can do the following: -/// -/// auto call_foo = []() -/// { -/// return foo(...); -/// } -/// -/// BoolArgsToTemplateArgsDispatcher::call(call_foo, b1, b2, ..., bn); - -template -struct BoolArgsToTemplateArgsDispatcher -{ - template - static auto call(Functor f, Args1&&... args) - { - return f.template operator()(std::forward(args)...); - } - - template - static auto call(Functor f, bool b, Args1&&... ar1) - { - if (b) - return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); - else - return BoolArgsToTemplateArgsDispatcher::call(f, std::forward(ar1)...); - } -}; - -} diff --git a/src/Interpreters/Aggregator.cpp b/src/Interpreters/Aggregator.cpp index d9e9c66e4e20..90f6fc7b9788 100644 --- a/src/Interpreters/Aggregator.cpp +++ b/src/Interpreters/Aggregator.cpp @@ -39,7 +39,6 @@ #include #include #include -#include #include @@ -1066,30 +1065,40 @@ void NO_INLINE Aggregator::executeImpl( { typename Method::State state(key_columns, key_sizes, aggregation_state_cache); - auto call_execute_impl_batch = [&]() - { - executeImplBatch(method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, overflow_row); - }; - - bool use_compiled_functions = false; -#if USE_EMBEDDED_COMPILER - use_compiled_functions = compiled_aggregate_functions_holder && !hasSparseArguments(aggregate_instructions); -#endif - if (!no_more_keys) { /// Prefetching doesn't make sense for small hash tables, because they fit in caches entirely. const bool prefetch = Method::State::has_cheap_key_calculation && params.enable_prefetch && (method.data.getBufferSizeInBytes() > min_bytes_for_prefetch); - BoolArgsToTemplateArgsDispatcher::call(call_execute_impl_batch, no_more_keys, use_compiled_functions, prefetch, all_keys_are_const); + +#if USE_EMBEDDED_COMPILER + if (compiled_aggregate_functions_holder && !hasSparseArguments(aggregate_instructions)) + { + if (prefetch) + executeImplBatch( + method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, all_keys_are_const, overflow_row); + else + executeImplBatch( + method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, all_keys_are_const, overflow_row); + } + else +#endif + { + if (prefetch) + executeImplBatch( + method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, all_keys_are_const, overflow_row); + else + executeImplBatch( + method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, all_keys_are_const, overflow_row); + } } else { - BoolArgsToTemplateArgsDispatcher::call(call_execute_impl_batch, no_more_keys, false, false, all_keys_are_const); + executeImplBatch(method, state, aggregates_pool, row_begin, row_end, aggregate_instructions, all_keys_are_const, overflow_row); } } -template +template void NO_INLINE Aggregator::executeImplBatch( Method & method, typename Method::State & state, @@ -1097,6 +1106,7 @@ void NO_INLINE Aggregator::executeImplBatch( size_t row_begin, size_t row_end, AggregateFunctionInstruction * aggregate_instructions, + bool all_keys_are_const, AggregateDataPtr overflow_row) const { using KeyHolder = decltype(state.getKeyHolder(0, std::declval())); @@ -1113,7 +1123,7 @@ void NO_INLINE Aggregator::executeImplBatch( /// For all rows. AggregateDataPtr place = aggregates_pool->alloc(0); - if constexpr (all_keys_are_const) + if (all_keys_are_const) { state.emplaceKey(method.data, 0, *aggregates_pool).setMapped(place); } @@ -1140,7 +1150,7 @@ void NO_INLINE Aggregator::executeImplBatch( } /// Optimization for special case when aggregating by 8bit key. - if constexpr (!no_more_keys && !all_keys_are_const && std::is_same_v) + if constexpr (!no_more_keys && std::is_same_v) { /// We use another method if there are aggregate functions with -Array combinator. bool has_arrays = false; @@ -1153,7 +1163,7 @@ void NO_INLINE Aggregator::executeImplBatch( } } - if (!has_arrays && !hasSparseArguments(aggregate_instructions)) + if (!has_arrays && !hasSparseArguments(aggregate_instructions) && !all_keys_are_const) { for (AggregateFunctionInstruction * inst = aggregate_instructions; inst->that; ++inst) { @@ -1184,7 +1194,7 @@ void NO_INLINE Aggregator::executeImplBatch( /// For all rows. size_t start, end; /// If all keys are const, key columns contain only 1 row. - if constexpr (all_keys_are_const) + if (all_keys_are_const) { start = 0; end = 1; @@ -1201,7 +1211,7 @@ void NO_INLINE Aggregator::executeImplBatch( if constexpr (!no_more_keys) { - if constexpr (prefetch && !all_keys_are_const && HasPrefetchMemberFunc) + if constexpr (prefetch && HasPrefetchMemberFunc) { if (i == row_begin + prefetching.iterationsToMeasure()) prefetch_look_ahead = prefetching.calcPrefetchLookAhead(); @@ -1273,7 +1283,7 @@ void NO_INLINE Aggregator::executeImplBatch( { /// If all keys are constant and this is new key /// we don't need to do anything and just skip the whole block. - if constexpr (all_keys_are_const) + if (all_keys_are_const) return; aggregate_data = overflow_row; } @@ -1299,7 +1309,7 @@ void NO_INLINE Aggregator::executeImplBatch( columns_data.emplace_back(getColumnData(inst->batch_arguments[argument_index])); } - if constexpr (all_keys_are_const) + if (all_keys_are_const) { auto add_into_aggregate_states_function_single_place = compiled_aggregate_functions_holder->compiled_aggregate_functions.add_into_aggregate_states_function_single_place; add_into_aggregate_states_function_single_place(row_begin, row_end, columns_data.data(), places[0]); @@ -1323,7 +1333,7 @@ void NO_INLINE Aggregator::executeImplBatch( AggregateFunctionInstruction * inst = aggregate_instructions + i; - if constexpr (all_keys_are_const) + if (all_keys_are_const) { if (inst->offsets) inst->batch_that->addBatchSinglePlace(inst->offsets[static_cast(row_begin) - 1], inst->offsets[row_end - 1], places[0] + inst->state_offset, inst->batch_arguments, aggregates_pool); diff --git a/src/Interpreters/Aggregator.h b/src/Interpreters/Aggregator.h index dc0391c4289a..d5b5abddd841 100644 --- a/src/Interpreters/Aggregator.h +++ b/src/Interpreters/Aggregator.h @@ -1297,7 +1297,7 @@ class Aggregator final AggregateDataPtr overflow_row) const; /// Specialization for a particular value no_more_keys. - template + template void executeImplBatch( Method & method, typename Method::State & state, @@ -1305,6 +1305,7 @@ class Aggregator final size_t row_begin, size_t row_end, AggregateFunctionInstruction * aggregate_instructions, + bool all_keys_are_const, AggregateDataPtr overflow_row) const; /// For case when there are no keys (all aggregate into one row). diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.reference b/tests/queries/0_stateless/02845_group_by_constant_keys.reference index 60e40ea54a7b..67cbdf0c025b 100644 --- a/tests/queries/0_stateless/02845_group_by_constant_keys.reference +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.reference @@ -2,3 +2,39 @@ 10000000 1 2 3 10000000 1 2 3 10000000 1 2 3 +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 data.Parquet +10 data.2.Parquet +10 data.1.Parquet +10 data.Parquet +10 data.2.Parquet +10 data.1.Parquet +10 data.Parquet +10 data.2.Parquet +10 data.1.Parquet +10 data.Parquet +10 data.2.Parquet +10 data.1.Parquet +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 +10 diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.sql b/tests/queries/0_stateless/02845_group_by_constant_keys.sql index 0223cf1df605..109a5a9a7308 100644 --- a/tests/queries/0_stateless/02845_group_by_constant_keys.sql +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.sql @@ -1,5 +1,29 @@ -SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; -SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 0; -SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions = 1; -SELECT count(number), 1 AS k1, 2 as k2, 3 as k3 FROM numbers_mt(10000000) GROUP BY k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 1; +select count(number), 1 AS k1, 2 as k2, 3 as k3 from numbers_mt(10000000) group by k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count(number), 1 AS k1, 2 as k2, 3 as k3 from numbers_mt(10000000) group by k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 0; +select count(number), 1 AS k1, 2 as k2, 3 as k3 from numbers_mt(10000000) group by k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions = 1; +select count(number), 1 AS k1, 2 as k2, 3 as k3 from numbers_mt(10000000) group by k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 1; + +drop table if exists test; +create table test (x UInt64) engine=File(Parquet); +set engine_file_allow_create_multiple_files = 1; +insert into test select * from numbers(10); +insert into test select * from numbers(10); +insert into test select * from numbers(10); + +select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; +select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; +select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; + +select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; +select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; +select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; + +select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; +select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; +select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; + +drop table test; From 27fb1b5ced804a37fee6c9fd110e029bb6cc01c5 Mon Sep 17 00:00:00 2001 From: Kruglov Pavel <48961922+Avogar@users.noreply.github.com> Date: Wed, 23 Aug 2023 22:18:30 +0200 Subject: [PATCH 07/10] Fix test --- tests/queries/0_stateless/02845_group_by_constant_keys.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.sql b/tests/queries/0_stateless/02845_group_by_constant_keys.sql index 109a5a9a7308..e16fd3fd1d20 100644 --- a/tests/queries/0_stateless/02845_group_by_constant_keys.sql +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.sql @@ -4,7 +4,7 @@ select count(number), 1 AS k1, 2 as k2, 3 as k3 from numbers_mt(10000000) group select count(number), 1 AS k1, 2 as k2, 3 as k3 from numbers_mt(10000000) group by k1, k2, k3 settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions = 1; drop table if exists test; -create table test (x UInt64) engine=File(Parquet); +create table test (x UInt64) engine=File(JSON); set engine_file_allow_create_multiple_files = 1; insert into test select * from numbers(10); insert into test select * from numbers(10); From 4b5784f7d021ca03b1759de944d777f780a6538d Mon Sep 17 00:00:00 2001 From: Kruglov Pavel <48961922+Avogar@users.noreply.github.com> Date: Wed, 6 Sep 2023 18:05:20 +0200 Subject: [PATCH 08/10] Fix test --- .../02845_group_by_constant_keys.sql | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.sql b/tests/queries/0_stateless/02845_group_by_constant_keys.sql index e16fd3fd1d20..053ad3ecdc4b 100644 --- a/tests/queries/0_stateless/02845_group_by_constant_keys.sql +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.sql @@ -10,20 +10,19 @@ insert into test select * from numbers(10); insert into test select * from numbers(10); insert into test select * from numbers(10); -select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; -select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; -select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; -select count() from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; +select count() from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count() from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; +select count() from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; +select count() from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; -select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; -select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; -select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; -select count(), _file from test group by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; +select count(), _file from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count(), _file from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; +select count(), _file from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; +select count(), _file from test group by _file order by _file settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; -select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; -select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; -select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; -select count() from test group by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; +select count() from test group by _file, _path order by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=0; +select count() from test group by _file, _path order by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=0; +select count() from test group by _file, _path order by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=0, compile_aggregate_expressions=1; +select count() from test group by _file, _path order by _file, _path settings optimize_group_by_constant_keys=1, enable_software_prefetch_in_aggregation=1, compile_aggregate_expressions=1; drop table test; - From b10639594abf7b3bf28a27b8974e221b3700d256 Mon Sep 17 00:00:00 2001 From: Kruglov Pavel <48961922+Avogar@users.noreply.github.com> Date: Wed, 6 Sep 2023 18:05:43 +0200 Subject: [PATCH 09/10] Update test reference --- .../02845_group_by_constant_keys.reference | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/queries/0_stateless/02845_group_by_constant_keys.reference b/tests/queries/0_stateless/02845_group_by_constant_keys.reference index 67cbdf0c025b..5473729c960b 100644 --- a/tests/queries/0_stateless/02845_group_by_constant_keys.reference +++ b/tests/queries/0_stateless/02845_group_by_constant_keys.reference @@ -14,18 +14,18 @@ 10 10 10 -10 data.Parquet -10 data.2.Parquet -10 data.1.Parquet -10 data.Parquet -10 data.2.Parquet -10 data.1.Parquet -10 data.Parquet -10 data.2.Parquet -10 data.1.Parquet -10 data.Parquet -10 data.2.Parquet -10 data.1.Parquet +10 data.1.JSON +10 data.2.JSON +10 data.JSON +10 data.1.JSON +10 data.2.JSON +10 data.JSON +10 data.1.JSON +10 data.2.JSON +10 data.JSON +10 data.1.JSON +10 data.2.JSON +10 data.JSON 10 10 10 From a075d03f01bbfc717304f9f10d953d5daa7c3c2e Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 15 Sep 2023 14:28:25 +0000 Subject: [PATCH 10/10] Fix --- src/Interpreters/Aggregator.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Interpreters/Aggregator.cpp b/src/Interpreters/Aggregator.cpp index 1811b1189ea7..e16064db713b 100644 --- a/src/Interpreters/Aggregator.cpp +++ b/src/Interpreters/Aggregator.cpp @@ -1281,10 +1281,6 @@ void NO_INLINE Aggregator::executeImplBatch( } else { - /// If all keys are constant and this is new key - /// we don't need to do anything and just skip the whole block. - if (all_keys_are_const) - return; aggregate_data = overflow_row; } }