From 454137f731e8f6ec055ce050e92c6b7dc3d17afa Mon Sep 17 00:00:00 2001 From: shiyer7474 Date: Thu, 26 Sep 2024 03:35:06 +0000 Subject: [PATCH] PR review feedback incorporated --- .../functions/other-functions.md | 15 +++++--- src/Functions/getSetting.cpp | 37 +++++++++++-------- .../01418_custom_settings.reference | 6 --- .../0_stateless/01418_custom_settings.sql | 9 ----- .../03234_get_setting_or_default.reference | 10 +++++ .../03234_get_setting_or_default.sql | 26 +++++++++++++ .../aspell-ignore/en/aspell-dict.txt | 2 +- 7 files changed, 69 insertions(+), 36 deletions(-) create mode 100644 tests/queries/0_stateless/03234_get_setting_or_default.reference create mode 100644 tests/queries/0_stateless/03234_get_setting_or_default.sql diff --git a/docs/en/sql-reference/functions/other-functions.md b/docs/en/sql-reference/functions/other-functions.md index 81c5800cef9..ca5cd7174ba 100644 --- a/docs/en/sql-reference/functions/other-functions.md +++ b/docs/en/sql-reference/functions/other-functions.md @@ -2789,33 +2789,38 @@ Result: - [Custom Settings](../../operations/settings/index.md#custom_settings) -## getSettingOrNull +## getSettingOrDefault -Returns the current value of a [custom setting](../../operations/settings/index.md#custom_settings) or returns NULL if setting is undefined. +Returns the current value of a [custom setting](../../operations/settings/index.md#custom_settings) or returns the default value specified in the 2nd argument if the custom setting is not set in the current profile. **Syntax** ```sql -getSettingOrNull('custom_setting'); +getSettingOrDefault('custom_setting', default_value); ``` **Parameter** - `custom_setting` — The setting name. [String](../data-types/string.md). +- `default_value` — Value to return if custom_setting is not set. Value may be of any data type or Null. **Returned value** -- The setting's current value or NULL if setting is undefined. +- The setting's current value or default_value if setting is not set. **Example** ```sql -SELECT getSetting('custom_xyz'); +SELECT getSettingOrDefault('custom_undef1', 'my_value'); +SELECT getSettingOrDefault('custom_undef2', 100); +SELECT getSettingOrDefault('custom_undef3', NULL); ``` Result: ``` +my_value +100 NULL ``` diff --git a/src/Functions/getSetting.cpp b/src/Functions/getSetting.cpp index aced1583ed3..d5973e25e77 100644 --- a/src/Functions/getSetting.cpp +++ b/src/Functions/getSetting.cpp @@ -19,22 +19,30 @@ namespace ErrorCodes namespace { -enum class ErrorHandling : uint8_t +enum class ErrorHandlingMode : uint8_t { - Exception, - Default, + Exception, /// Raise exception if setting not found (getSetting()) + Default, /// Return default value if setting not found (getSettingOrDefault()) }; +struct NameGetSetting{ static constexpr auto name = "getSetting"; }; + +struct NameGetSettingOrDefault{ static constexpr auto name = "getSettingOrDefault"; }; + /// Get the value of a setting. +template class FunctionGetSetting : public IFunction, WithContext { public: - explicit FunctionGetSetting(ContextPtr context_, String name_, ErrorHandling error_handling_) : WithContext(context_), name(name_), error_handling(error_handling_) {} + static constexpr auto name = Name::name; + + static FunctionPtr create(ContextPtr context_) { return std::make_shared(context_); } + explicit FunctionGetSetting(ContextPtr context_) : WithContext(context_) {} String getName() const override { return name; } bool isDeterministic() const override { return false; } bool isSuitableForShortCircuitArgumentsExecution(const DataTypesWithConstInfo & /*arguments*/) const override { return false; } - size_t getNumberOfArguments() const override { return ( name == "getSettingOrDefault" ? 2 : 1 ); } + size_t getNumberOfArguments() const override { return ( mode == ErrorHandlingMode::Default ? 2 : 1 ); } ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {0,1}; } DataTypePtr getReturnTypeImpl(const ColumnsWithTypeAndName & arguments) const override @@ -61,7 +69,7 @@ private: throw Exception(ErrorCodes::ILLEGAL_COLUMN, "The argument of function {} should be a constant string with the name of a setting", String{name}); - if (arguments.size() == 2) + if (mode == ErrorHandlingMode::Default) { const auto * default_value_column = arguments[1].column.get(); if (!default_value_column || default_value_column->size() != 1) @@ -79,9 +87,9 @@ private: } catch(...) { - if (error_handling == ErrorHandling::Exception) + if (mode == ErrorHandlingMode::Exception) throw; - else if (error_handling == ErrorHandling::Default) + else if (mode == ErrorHandlingMode::Default) { auto default_value_column = arguments[1].column; @@ -90,22 +98,21 @@ private: } return setting_value; } - String name; - ErrorHandling error_handling; }; } +using FunctionGetSettingWithException = FunctionGetSetting; +using FunctionGetSettingWithDefault = FunctionGetSetting; + REGISTER_FUNCTION(GetSetting) { - factory.registerFunction("getSetting", - [](ContextPtr context){ return std::make_shared(context, "getSetting", ErrorHandling::Exception); }); + factory.registerFunction(); } + REGISTER_FUNCTION(GetSettingOrDefault) { - factory.registerFunction("getSettingOrDefault", - [](ContextPtr context){ return std::make_shared(context, "getSettingOrDefault", ErrorHandling::Default); }); + factory.registerFunction(); } - } diff --git a/tests/queries/0_stateless/01418_custom_settings.reference b/tests/queries/0_stateless/01418_custom_settings.reference index f1b37dd2088..923d43077d8 100644 --- a/tests/queries/0_stateless/01418_custom_settings.reference +++ b/tests/queries/0_stateless/01418_custom_settings.reference @@ -37,9 +37,3 @@ custom_null NULL \N Nullable(Nothing) custom_null NULL CREATE SETTINGS PROFILE `s2_01418` SETTINGS custom_null = NULL -my_default_value -test -\N -50 -1 -1 diff --git a/tests/queries/0_stateless/01418_custom_settings.sql b/tests/queries/0_stateless/01418_custom_settings.sql index e573c615963..f121dba0597 100644 --- a/tests/queries/0_stateless/01418_custom_settings.sql +++ b/tests/queries/0_stateless/01418_custom_settings.sql @@ -61,12 +61,3 @@ SELECT name, value FROM system.settings WHERE name = 'custom_null'; CREATE SETTINGS PROFILE s2_01418 SETTINGS custom_null = NULL; SHOW CREATE SETTINGS PROFILE s2_01418; DROP SETTINGS PROFILE s2_01418; - -SELECT getSetting('custom_xyz') as v; -- { serverError UNKNOWN_SETTING } -- Setting not found. -SELECT getSettingOrDefault('custom_xyz','my_default_value') as v; -SELECT getSettingOrDefault('custom_compound.identifier.v1','should not be seen') as v; -SELECT getSettingOrDefault('custom_xyz', NULL) as v; -SELECT getSettingOrDefault('custom_xyz', 50) as v; -SELECT getSettingOrDefault(1, 'def') as v; -- { serverError ILLEGAL_TYPE_OF_ARGUMENT } -SELECT count(*) FROM numbers(10) WHERE number = getSettingOrDefault('custom_role',5); -SELECT isNull(getSettingOrDefault('custom_xyz',NULL)) as v; diff --git a/tests/queries/0_stateless/03234_get_setting_or_default.reference b/tests/queries/0_stateless/03234_get_setting_or_default.reference new file mode 100644 index 00000000000..0b47065a07b --- /dev/null +++ b/tests/queries/0_stateless/03234_get_setting_or_default.reference @@ -0,0 +1,10 @@ +value_a +value_b +\N +5 +default_e +500 +\N +1 +1 +backup diff --git a/tests/queries/0_stateless/03234_get_setting_or_default.sql b/tests/queries/0_stateless/03234_get_setting_or_default.sql new file mode 100644 index 00000000000..0842794ea67 --- /dev/null +++ b/tests/queries/0_stateless/03234_get_setting_or_default.sql @@ -0,0 +1,26 @@ +SET custom_a = 'value_a'; +SET custom_b = 'value_b'; +SET custom_c = null; +SET custom_d = 5; + + + +SELECT getSettingOrDefault('custom_a','default_a'); +SELECT getSettingOrDefault('custom_b','default_b'); +SELECT getSettingOrDefault('custom_c','default_c'); +SELECT getSettingOrDefault('custom_d','default_d'); + +SELECT getSetting('custom_e'); -- { serverError UNKNOWN_SETTING } + +SELECT getSettingOrDefault('custom_e','default_e'); +SELECT getSettingOrDefault('custom_e',500); +SELECT getSettingOrDefault('custom_e',null); +SELECT isNull(getSettingOrDefault('custom_e',null)); + +SELECT getSettingOrDefault('custom_e'); -- { serverError NUMBER_OF_ARGUMENTS_DOESNT_MATCH } +SELECT getSettingOrDefault(115,'name should be string'); -- { serverError ILLEGAL_TYPE_OF_ARGUMENT } + +SELECT count(*) FROM numbers(10) WHERE number = getSettingOrDefault('custom_e',5); + +SET custom_e_backup = 'backup'; +SELECT getSettingOrDefault('custom_e', getSetting('custom_e_backup')); diff --git a/utils/check-style/aspell-ignore/en/aspell-dict.txt b/utils/check-style/aspell-ignore/en/aspell-dict.txt index eedf70a47ac..2b57b990f1d 100644 --- a/utils/check-style/aspell-ignore/en/aspell-dict.txt +++ b/utils/check-style/aspell-ignore/en/aspell-dict.txt @@ -1734,7 +1734,7 @@ getMacro getOSKernelVersion getServerPort getSetting -getSettingOrNull +getSettingOrDefault getSizeOfEnumType getSubcolumn getTypeSerializationStreams