Re: set/reset issues in create function and set_config

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)