From 9794a193cfb88d7a49b12b9a60986884bf3ebfda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Fri, 24 May 2024 15:05:49 +0200 Subject: [PATCH] Rename aggregate_function_group_array_has_limit_size --- .../AggregateFunctionGroupArray.cpp | 11 ++++++----- src/Core/ServerSettings.h | 3 ++- src/Core/SettingsEnums.cpp | 5 +++++ src/Core/SettingsEnums.h | 8 ++++++++ .../configs/group_array_max_element_size.xml | 2 +- .../integration/test_group_array_element_size/test.py | 8 ++++---- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/AggregateFunctions/AggregateFunctionGroupArray.cpp b/src/AggregateFunctions/AggregateFunctionGroupArray.cpp index d4fb7afcb78..c21b1d376d9 100644 --- a/src/AggregateFunctions/AggregateFunctionGroupArray.cpp +++ b/src/AggregateFunctions/AggregateFunctionGroupArray.cpp @@ -753,10 +753,11 @@ size_t getMaxArraySize() return 0xFFFFFF; } -bool hasLimitArraySize() +bool discardOnLimitReached() { if (auto context = Context::getGlobalContextInstance()) - return context->getServerSettings().aggregate_function_group_array_has_limit_size; + return context->getServerSettings().aggregate_function_group_array_action_when_limit_is_reached + == GroupArrayActionWhenLimitReached::DISCARD; return false; } @@ -767,7 +768,7 @@ AggregateFunctionPtr createAggregateFunctionGroupArray( { assertUnary(name, argument_types); - bool limit_size = hasLimitArraySize(); + bool has_limit = discardOnLimitReached(); UInt64 max_elems = getMaxArraySize(); if (parameters.empty()) @@ -784,14 +785,14 @@ AggregateFunctionPtr createAggregateFunctionGroupArray( (type == Field::Types::UInt64 && parameters[0].get() == 0)) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Parameter for aggregate function {} should be positive number", name); - limit_size = true; + has_limit = true; max_elems = parameters[0].get(); } else throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, "Incorrect number of parameters for aggregate function {}, should be 0 or 1", name); - if (!limit_size) + if (!has_limit) { if (Tlast) throw Exception(ErrorCodes::BAD_ARGUMENTS, "groupArrayLast make sense only with max_elems (groupArrayLast(max_elems)())"); diff --git a/src/Core/ServerSettings.h b/src/Core/ServerSettings.h index ea0b155b22d..45f235116ab 100644 --- a/src/Core/ServerSettings.h +++ b/src/Core/ServerSettings.h @@ -3,6 +3,7 @@ #include #include +#include namespace Poco::Util @@ -51,7 +52,7 @@ namespace DB M(UInt64, max_temporary_data_on_disk_size, 0, "The maximum amount of storage that could be used for external aggregation, joins or sorting., ", 0) \ M(String, temporary_data_in_cache, "", "Cache disk name for temporary data.", 0) \ M(UInt64, aggregate_function_group_array_max_element_size, 0xFFFFFF, "Max array element size in bytes for groupArray function. This limit is checked at serialization and help to avoid large state size.", 0) \ - M(Bool, aggregate_function_group_array_has_limit_size, false, "When the max array element size is exceeded, a `Too large array size` exception will be thrown by default. When set to true, no exception will be thrown, and the excess elements will be discarded.", 0) \ + M(GroupArrayActionWhenLimitReached, aggregate_function_group_array_action_when_limit_is_reached, GroupArrayActionWhenLimitReached::THROW, "Action to execute when max array element size is exceeded in groupArray: `throw` exception, or `discard` extra values", 0) \ M(UInt64, max_server_memory_usage, 0, "Maximum total memory usage of the server in bytes. Zero means unlimited.", 0) \ M(Double, max_server_memory_usage_to_ram_ratio, 0.9, "Same as max_server_memory_usage but in to RAM ratio. Allows to lower max memory on low-memory systems.", 0) \ M(UInt64, merges_mutations_memory_usage_soft_limit, 0, "Maximum total memory usage for merges and mutations in bytes. Zero means unlimited.", 0) \ diff --git a/src/Core/SettingsEnums.cpp b/src/Core/SettingsEnums.cpp index 0caf6e8d609..05985316566 100644 --- a/src/Core/SettingsEnums.cpp +++ b/src/Core/SettingsEnums.cpp @@ -229,4 +229,9 @@ IMPLEMENT_SETTING_ENUM(SQLSecurityType, ErrorCodes::BAD_ARGUMENTS, {{"DEFINER", SQLSecurityType::DEFINER}, {"INVOKER", SQLSecurityType::INVOKER}, {"NONE", SQLSecurityType::NONE}}) + +IMPLEMENT_SETTING_ENUM( + GroupArrayActionWhenLimitReached, + ErrorCodes::BAD_ARGUMENTS, + {{"throw", GroupArrayActionWhenLimitReached::THROW}, {"discard", GroupArrayActionWhenLimitReached::DISCARD}}) } diff --git a/src/Core/SettingsEnums.h b/src/Core/SettingsEnums.h index ab163ba96a3..575cd8700c8 100644 --- a/src/Core/SettingsEnums.h +++ b/src/Core/SettingsEnums.h @@ -370,4 +370,12 @@ DECLARE_SETTING_ENUM(SchemaInferenceMode) DECLARE_SETTING_ENUM_WITH_RENAME(DateTimeOverflowBehavior, FormatSettings::DateTimeOverflowBehavior) DECLARE_SETTING_ENUM(SQLSecurityType) + +enum class GroupArrayActionWhenLimitReached : uint8_t +{ + THROW, + DISCARD +}; +DECLARE_SETTING_ENUM(GroupArrayActionWhenLimitReached) + } diff --git a/tests/integration/test_group_array_element_size/configs/group_array_max_element_size.xml b/tests/integration/test_group_array_element_size/configs/group_array_max_element_size.xml index 80409d3e18b..32d5d131a44 100644 --- a/tests/integration/test_group_array_element_size/configs/group_array_max_element_size.xml +++ b/tests/integration/test_group_array_element_size/configs/group_array_max_element_size.xml @@ -1,4 +1,4 @@ 10 - false + throw diff --git a/tests/integration/test_group_array_element_size/test.py b/tests/integration/test_group_array_element_size/test.py index 1eb7647d734..90b2712ffbf 100644 --- a/tests/integration/test_group_array_element_size/test.py +++ b/tests/integration/test_group_array_element_size/test.py @@ -80,8 +80,8 @@ def test_limit_size(started_cluster): node2.replace_in_config( "/etc/clickhouse-server/config.d/group_array_max_element_size.xml", - "false", - "true", + "throw", + "discard", ) node2.restart_clickhouse() @@ -91,8 +91,8 @@ def test_limit_size(started_cluster): node2.replace_in_config( "/etc/clickhouse-server/config.d/group_array_max_element_size.xml", - "true", - "false", + "discard", + "throw", ) node2.restart_clickhouse()