I wrote:
> Nathan Bossart <nathandboss...@gmail.com> writes:
>> Another option might be to introduce a new GUC flag or source for anything
>> we want to bypass the check (perhaps with the stipulation that it must also
>> be marked PGC_INTERNAL).
> A new GUC flag seems like a promising approach, and better than
> giving a blanket exemption to PGC_INTERNAL. At least for
> is_superuser, there's no visible value in restricting which
> SetConfigOption calls can change it; they'd all need the escape
> hatch.
Here's a draft patch to fix it with a flag, now with regression tests.
Also, now that the error depends on which parameter we're talking
about, I thought it best to include the parameter name in the error
and to re-word it to be more like all the other can't-set-this-now
errors just below it. I'm half tempted to change the errcode and
set_config_option return value to match the others too, ie
ERRCODE_CANT_CHANGE_RUNTIME_PARAM and "return 0" not -1.
I don't think the existing choices here are very well thought
through, and they're certainly inconsistent with a lot of
otherwise-similar-seeming refusals in set_config_option.
regards, tom lane
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0c593b81b4..7dd7b2ec4a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3428,6 +3428,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,26 +3445,20 @@ 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")));
+ errmsg("parameter \"%s\" cannot be set during a parallel operation",
+ name)));
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;
- }
- else
- record = handle;
-
/*
* Check if the option can be set at this time. See guc.h for the precise
* rules.
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c0a52cdcc3..1c4392dd22 100644
--- 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
},
¤t_role_is_superuser,
false,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 4129ea37ee..ba6883ae8f 100644
--- 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 68a629619a..a61c138bd0 100644
--- 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 1a0e7f144d..22384b5ad8 100644
--- 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;
