Обсуждение: set/reset issues in create function and set_config

Hello,

It looks like set_config does set-to-default if it's called with second argument being null, but this is undocumented.
Please see the docs patch to fix it.

Also, SET to DEFAULT or RESET in CREATE FUNCTION is accepted but does nothing.
It wouldn't be a problem, but documentation to SET command says:

> If SET LOCAL is used within a function that has a SET option for the same variable (see CREATE FUNCTION), the effects of the SET LOCAL command disappear at function exit

This doesn't work with CREATE FUNCTION ... SET ... DEFAULT ....
What do you think of just disallowing it? Patch attached as well.

Best regards,
  Alexey

At Tue, 24 Apr 2018 16:32:08 +0100, Alexey Bashtanov <bashtanov@imap.cc> wrote in
<a7c1f0cb-1031-abf8-e4cc-13cf781d6563@imap.cc>
> Hello,
> 
> It looks like set_config does set-to-default if it's called with
> second argument being null, but this is undocumented.
> Please see the docs patch to fix it.

Agreed.

> Also, SET to DEFAULT or RESET in CREATE FUNCTION is accepted but does
> nothing.
> It wouldn't be a problem, but documentation to SET command says:
> 
> > If SET LOCAL is used within a function that has a SET option for the
> > same variable (see CREATE FUNCTION
> > <https://www.postgresql.org/docs/9.4/static/sql-createfunction.html>),
> > the effects of the SET LOCAL command disappear at function exit
> 
> This doesn't work with CREATE FUNCTION ... SET ... DEFAULT ....
> What do you think of just disallowing it? Patch attached as well.

SET .. DEFAULT and RESET are defined as follows.

https://www.postgresql.org/docs/10/static/sql-reset.html

> The default value is defined as the value that the parameter
> would have had, if no SET had ever been issued for it in the
> current session.

Precisely following this statement, SET ... TO DEFAULT changes
the variable to the session default temporarily. For example, the
attached patch (of course, just a PoC) does that.

=# create or replace function f() returns record as $$ select name, setting, reset_val from pg_settings where name =
'log_min_messages';$$ set log_min_messages to default language sql;
 
=# set log_min_messages = debug5;
=# show log_min_messages;
 log_min_messages 
------------------
 debug5
(1 row)
=# set log_min_messages = debug5;
SET
postgres=# select f();
                 f                  
------------------------------------
 (log_min_messages,warning,warning)
(1 row)
=# show log_min_messages;
 log_min_messages 
------------------
 debug5
(1 row)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 3c74873eeb..f04304c244 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -624,10 +624,7 @@ update_proconfig_value(ArrayType *a, List *set_items)
         {
             char       *valuestr = ExtractSetVariableArgs(sstmt);
 
-            if (valuestr)
-                a = GUCArrayAdd(a, sstmt->name, valuestr);
-            else                /* RESET */
-                a = GUCArrayDelete(a, sstmt->name);
+            a = GUCArrayAdd(a, sstmt->name, valuestr);
         }
     }
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 44dfa92722..200acd694b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -9709,7 +9709,13 @@ ProcessGUCArray(ArrayType *array,
             continue;
         }
 
-        (void) set_config_option(name, value,
+        if (value && strcmp(value, "default") == 0)
+            (void) set_config_option(name, NULL,
+                                     context, source,
+                                     action, true, 0, false);
+
+        else
+            (void) set_config_option(name, value,
                                  context, source,
                                  action, true, 0, false);
 
@@ -9734,7 +9740,6 @@ GUCArrayAdd(ArrayType *array, const char *name, const char *value)
     ArrayType  *a;
 
     Assert(name);
-    Assert(value);
 
     /* test if the option is valid and we're allowed to set it */
     (void) validate_option_array_item(name, value, false);
@@ -9745,7 +9750,11 @@ GUCArrayAdd(ArrayType *array, const char *name, const char *value)
         name = record->name;
 
     /* build new item for array */
-    newval = psprintf("%s=%s", name, value);
+    if (value)
+        newval = psprintf("%s=%s", name, value);
+    else
+        newval = psprintf("%s=default", name);
+        
     datum = CStringGetTextDatum(newval);
 
     if (array)