PR review feedback incorporated

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
 ```
 

src/Functions/getSetting.cpp

@@ -19,22 +19,30 @@ namespace ErrorCodes
 namespace
 {
 
-enum class ErrorHandling : uint8_t
+enum class ErrorHandlingMode : uint8_t
 {
-    Exception,
+    Exception,  /// Raise exception if setting not found (getSetting())
-    Default,
+    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 <typename Name, ErrorHandlingMode mode>
 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<FunctionGetSetting>(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<NameGetSetting, ErrorHandlingMode::Exception>;
+using FunctionGetSettingWithDefault = FunctionGetSetting<NameGetSettingOrDefault, ErrorHandlingMode::Default>;
+
 REGISTER_FUNCTION(GetSetting)
 {
-    factory.registerFunction("getSetting",
+    factory.registerFunction<FunctionGetSettingWithException>();
-        [](ContextPtr context){ return std::make_shared<FunctionGetSetting>(context, "getSetting", ErrorHandling::Exception); });
 }
 
+
 REGISTER_FUNCTION(GetSettingOrDefault)
 {
-    factory.registerFunction("getSettingOrDefault",
+    factory.registerFunction<FunctionGetSettingWithDefault>();
-        [](ContextPtr context){ return std::make_shared<FunctionGetSetting>(context, "getSettingOrDefault", ErrorHandling::Default); });
 }
-
 }

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

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;

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

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'));

utils/check-style/aspell-ignore/en/aspell-dict.txt

@@ -1734,7 +1734,7 @@ getMacro
 getOSKernelVersion
 getServerPort
 getSetting
-getSettingOrNull
+getSettingOrDefault
 getSizeOfEnumType
 getSubcolumn
 getTypeSerializationStreams