Allow adjusting session_authorization and role in parallel workers.

The code intends to allow GUCs to be set within parallel workers
via function SET clauses, but not otherwise.  However, doing so fails
for "session_authorization" and "role", because the assign hooks for
those attempt to set the subsidiary "is_superuser" GUC, and that call
falls foul of the "not otherwise" prohibition.  We can't switch to
using GUC_ACTION_SAVE for this, so instead add a new GUC variable
flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set
anyway.  (This is okay because is_superuser has context PGC_INTERNAL
and thus only hard-wired calls can change it.  We'd need more thought
before applying the flag to other GUCs; but maybe there are other
use-cases.)  This isn't the prettiest fix perhaps, but other
alternatives we thought of would be much more invasive.

While here, correct a thinko in commit 059de3ca4: when rejecting
a GUC setting within a parallel worker, we should return 0 not -1
if the ereport doesn't longjmp.  (This seems to have no consequences
right now because no caller cares, but it's inconsistent.)  Improve
the comments to try to forestall future confusion of the same kind.

Despite the lack of field complaints, this seems worth back-patching.
Thanks to Nathan Bossart for the idea to invent a new flag,
and for review.

Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0c593b81b4e333d5fb3701b8149cd629409377bf..13527fc258f70a4092f9558e9e628fa4c4c7e8f3 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3324,10 +3324,12 @@ parse_and_validate_value(struct config_generic *record,
  *
  * Return value:
  * +1: the value is valid and was successfully applied.
- * 0:  the name or value is invalid (but see below).
- * -1: the value was not applied because of context, priority, or changeVal.
+ * 0:  the name or value is invalid, or it's invalid to try to set
+ *     this GUC now; but elevel was less than ERROR (see below).
+ * -1: no error detected, but the value was not applied, either
+ *     because changeVal is false or there is some overriding setting.
  *
- * If there is an error (non-existing option, invalid value) then an
+ * If there is an error (non-existing option, invalid value, etc) then an
  * ereport(ERROR) is thrown *unless* this is called for a source for which
  * we don't want an ERROR (currently, those are defaults, the config file,
  * and per-database or per-user settings, as well as callers who specify
@@ -3390,8 +3392,11 @@ set_config_option_ext(const char *name, const char *value,
 
 
 /*
- * set_config_with_handle: takes an optional 'handle' argument, which can be
- * obtained by the caller from get_config_handle().
+ * set_config_with_handle: sets option `name' to given value.
+ *
+ * This API adds the ability to pass a 'handle' argument, which can be
+ * obtained by the caller from get_config_handle().  NULL has no effect,
+ * but a non-null value avoids the need to search the GUC tables.
  *
  * This should be used by callers which repeatedly set the same config
  * option(s), and want to avoid the overhead of a hash lookup each time.
@@ -3428,6 +3433,16 @@ set_config_with_handle(const char *name, config_handle *handle,
            elevel = ERROR;
    }
 
+   /* if handle is specified, no need to look up option */
+   if (!handle)
+   {
+       record = find_option(name, true, false, elevel);
+       if (record == NULL)
+           return 0;
+   }
+   else
+       record = handle;
+
    /*
     * GUC_ACTION_SAVE changes are acceptable during a parallel operation,
     * because the current worker will also pop the change.  We're probably
@@ -3435,25 +3450,19 @@ set_config_with_handle(const char *name, config_handle *handle,
     * body should observe the change, and peer workers do not share in the
     * execution of a function call started by this worker.
     *
+    * Also allow normal setting if the GUC is marked GUC_ALLOW_IN_PARALLEL.
+    *
     * Other changes might need to affect other workers, so forbid them.
     */
-   if (IsInParallelMode() && changeVal && action != GUC_ACTION_SAVE)
+   if (IsInParallelMode() && changeVal && action != GUC_ACTION_SAVE &&
+       (record->flags & GUC_ALLOW_IN_PARALLEL) == 0)
    {
        ereport(elevel,
                (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
-                errmsg("cannot set parameters during a parallel operation")));
-       return -1;
-   }
-
-   /* if handle is specified, no need to look up option */
-   if (!handle)
-   {
-       record = find_option(name, true, false, elevel);
-       if (record == NULL)
-           return 0;
+                errmsg("parameter \"%s\" cannot be set during a parallel operation",
+                       name)));
+       return 0;
    }
-   else
-       record = handle;
 
    /*
     * Check if the option can be set at this time. See guc.h for the precise

diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 289dea7878a6dbb2e932e56ffcbd4cf9e5aba060..636780673b8acd780dd5fde7629ee53f34475f85 100644 (file)
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1015,7 +1015,7 @@ struct config_bool ConfigureNamesBool[] =
        {"is_superuser", PGC_INTERNAL, UNGROUPED,
            gettext_noop("Shows whether the current user is a superuser."),
            NULL,
-           GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+           GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_ALLOW_IN_PARALLEL
        },
        &current_role_is_superuser,
        false,

diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 4129ea37eec39af06c2f127e31d38e0dae3e37ee..ba6883ae8fc50d383286b9fdcd9df0a354be865e 100644 (file)
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -223,6 +223,7 @@ typedef enum
 #define GUC_DISALLOW_IN_AUTO_FILE \
                               0x002000 /* can't set in PG_AUTOCONF_FILENAME */
 #define GUC_RUNTIME_COMPUTED   0x004000 /* delay processing in 'postgres -C' */
+#define GUC_ALLOW_IN_PARALLEL  0x008000 /* allow setting in parallel mode */
 
 #define GUC_UNIT_KB             0x01000000 /* value is in kilobytes */
 #define GUC_UNIT_BLOCKS         0x02000000 /* value is in blocks */

diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index 68a629619a6eb4bd4725a6bfb1c485863c93e1e1..a61c138bd0a76ca507617cdc1ca67dcaa2f3efc9 100644 (file)
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -1347,3 +1347,36 @@ select current_setting('session_authorization');
 (1 row)
 
 rollback;
+-- test that function option SET ROLE works in parallel workers.
+create role regress_parallel_worker;
+create function set_and_report_role() returns text as
+  $$ select current_setting('role') $$ language sql parallel safe
+  set role = regress_parallel_worker;
+create function set_role_and_error(int) returns int as
+  $$ select 1 / $1 $$ language sql parallel safe
+  set role = regress_parallel_worker;
+set debug_parallel_query = 0;
+select set_and_report_role();
+   set_and_report_role   
+-------------------------
+ regress_parallel_worker
+(1 row)
+
+select set_role_and_error(0);
+ERROR:  division by zero
+CONTEXT:  SQL function "set_role_and_error" statement 1
+set debug_parallel_query = 1;
+select set_and_report_role();
+   set_and_report_role   
+-------------------------
+ regress_parallel_worker
+(1 row)
+
+select set_role_and_error(0);
+ERROR:  division by zero
+CONTEXT:  SQL function "set_role_and_error" statement 1
+parallel worker
+reset debug_parallel_query;
+drop function set_and_report_role();
+drop function set_role_and_error(int);
+drop role regress_parallel_worker;

diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index 1a0e7f144d02e90e626296c27eda1e678a409b44..22384b5ad85bcca830b58b16ead9abb09ddfb445 100644 (file)
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -520,3 +520,26 @@ select current_setting('session_authorization');
 set debug_parallel_query = 1;
 select current_setting('session_authorization');
 rollback;
+
+-- test that function option SET ROLE works in parallel workers.
+create role regress_parallel_worker;
+
+create function set_and_report_role() returns text as
+  $$ select current_setting('role') $$ language sql parallel safe
+  set role = regress_parallel_worker;
+
+create function set_role_and_error(int) returns int as
+  $$ select 1 / $1 $$ language sql parallel safe
+  set role = regress_parallel_worker;
+
+set debug_parallel_query = 0;
+select set_and_report_role();
+select set_role_and_error(0);
+set debug_parallel_query = 1;
+select set_and_report_role();
+select set_role_and_error(0);
+reset debug_parallel_query;
+
+drop function set_and_report_role();
+drop function set_role_and_error(int);
+drop role regress_parallel_worker;