Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit364de74

Browse files
committed
Allow adjusting session_authorization and role in parallel workers.
The code intends to allow GUCs to be set within parallel workersvia function SET clauses, but not otherwise. However, doing so failsfor "session_authorization" and "role", because the assign hooks forthose attempt to set the subsidiary "is_superuser" GUC, and that callfalls foul of the "not otherwise" prohibition. We can't switch tousing GUC_ACTION_SAVE for this, so instead add a new GUC variableflag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to setanyway. (This is okay because is_superuser has context PGC_INTERNALand thus only hard-wired calls can change it. We'd need more thoughtbefore applying the flag to other GUCs; but maybe there are otheruse-cases.) This isn't the prettiest fix perhaps, but otheralternatives we thought of would be much more invasive.While here, correct a thinko in commit059de3c: when rejectinga GUC setting within a parallel worker, we should return 0 not -1if the ereport doesn't longjmp. (This seems to have no consequencesright now because no caller cares, but it's inconsistent.) Improvethe 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
1 parent0868d7a commit364de74

File tree

5 files changed

+85
-19
lines changed

5 files changed

+85
-19
lines changed

‎src/backend/utils/misc/guc.c

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3324,10 +3324,12 @@ parse_and_validate_value(struct config_generic *record,
33243324
*
33253325
* Return value:
33263326
*+1: the value is valid and was successfully applied.
3327-
*0:the name or value is invalid (but see below).
3328-
*-1: the value was not applied because of context, priority, or changeVal.
3327+
*0:the name or value is invalid, or it's invalid to try to set
3328+
*this GUC now; but elevel was less than ERROR (see below).
3329+
*-1: no error detected, but the value was not applied, either
3330+
*because changeVal is false or there is some overriding setting.
33293331
*
3330-
* If there is an error (non-existing option, invalid value) then an
3332+
* If there is an error (non-existing option, invalid value, etc) then an
33313333
* ereport(ERROR) is thrown *unless* this is called for a source for which
33323334
* we don't want an ERROR (currently, those are defaults, the config file,
33333335
* 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,
33903392

33913393

33923394
/*
3393-
* set_config_with_handle: takes an optional 'handle' argument, which can be
3394-
* obtained by the caller from get_config_handle().
3395+
* set_config_with_handle: sets option `name' to given value.
3396+
*
3397+
* This API adds the ability to pass a 'handle' argument, which can be
3398+
* obtained by the caller from get_config_handle(). NULL has no effect,
3399+
* but a non-null value avoids the need to search the GUC tables.
33953400
*
33963401
* This should be used by callers which repeatedly set the same config
33973402
* option(s), and want to avoid the overhead of a hash lookup each time.
@@ -3428,32 +3433,36 @@ set_config_with_handle(const char *name, config_handle *handle,
34283433
elevel=ERROR;
34293434
}
34303435

3436+
/* if handle is specified, no need to look up option */
3437+
if (!handle)
3438+
{
3439+
record=find_option(name, true, false,elevel);
3440+
if (record==NULL)
3441+
return0;
3442+
}
3443+
else
3444+
record=handle;
3445+
34313446
/*
34323447
* GUC_ACTION_SAVE changes are acceptable during a parallel operation,
34333448
* because the current worker will also pop the change. We're probably
34343449
* dealing with a function having a proconfig entry. Only the function's
34353450
* body should observe the change, and peer workers do not share in the
34363451
* execution of a function call started by this worker.
34373452
*
3453+
* Also allow normal setting if the GUC is marked GUC_ALLOW_IN_PARALLEL.
3454+
*
34383455
* Other changes might need to affect other workers, so forbid them.
34393456
*/
3440-
if (IsInParallelMode()&&changeVal&&action!=GUC_ACTION_SAVE)
3457+
if (IsInParallelMode()&&changeVal&&action!=GUC_ACTION_SAVE&&
3458+
(record->flags&GUC_ALLOW_IN_PARALLEL)==0)
34413459
{
34423460
ereport(elevel,
34433461
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
3444-
errmsg("cannot set parameters during a parallel operation")));
3445-
return-1;
3446-
}
3447-
3448-
/* if handle is specified, no need to look up option */
3449-
if (!handle)
3450-
{
3451-
record=find_option(name, true, false,elevel);
3452-
if (record==NULL)
3453-
return0;
3462+
errmsg("parameter \"%s\" cannot be set during a parallel operation",
3463+
name)));
3464+
return0;
34543465
}
3455-
else
3456-
record=handle;
34573466

34583467
/*
34593468
* Check if the option can be set at this time. See guc.h for the precise

‎src/backend/utils/misc/guc_tables.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ struct config_bool ConfigureNamesBool[] =
10151015
{"is_superuser",PGC_INTERNAL,UNGROUPED,
10161016
gettext_noop("Shows whether the current user is a superuser."),
10171017
NULL,
1018-
GUC_REPORT |GUC_NO_SHOW_ALL |GUC_NO_RESET_ALL |GUC_NOT_IN_SAMPLE |GUC_DISALLOW_IN_FILE
1018+
GUC_REPORT |GUC_NO_SHOW_ALL |GUC_NO_RESET_ALL |GUC_NOT_IN_SAMPLE |GUC_DISALLOW_IN_FILE |GUC_ALLOW_IN_PARALLEL
10191019
},
10201020
&current_role_is_superuser,
10211021
false,

‎src/include/utils/guc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ typedef enum
223223
#defineGUC_DISALLOW_IN_AUTO_FILE \
224224
0x002000/* can't set in PG_AUTOCONF_FILENAME */
225225
#defineGUC_RUNTIME_COMPUTED 0x004000/* delay processing in 'postgres -C' */
226+
#defineGUC_ALLOW_IN_PARALLEL 0x008000/* allow setting in parallel mode */
226227

227228
#defineGUC_UNIT_KB 0x01000000/* value is in kilobytes */
228229
#defineGUC_UNIT_BLOCKS 0x02000000/* value is in blocks */

‎src/test/regress/expected/select_parallel.out

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,3 +1347,36 @@ select current_setting('session_authorization');
13471347
(1 row)
13481348

13491349
rollback;
1350+
-- test that function option SET ROLE works in parallel workers.
1351+
create role regress_parallel_worker;
1352+
create function set_and_report_role() returns text as
1353+
$$ select current_setting('role') $$ language sql parallel safe
1354+
set role = regress_parallel_worker;
1355+
create function set_role_and_error(int) returns int as
1356+
$$ select 1 / $1 $$ language sql parallel safe
1357+
set role = regress_parallel_worker;
1358+
set debug_parallel_query = 0;
1359+
select set_and_report_role();
1360+
set_and_report_role
1361+
-------------------------
1362+
regress_parallel_worker
1363+
(1 row)
1364+
1365+
select set_role_and_error(0);
1366+
ERROR: division by zero
1367+
CONTEXT: SQL function "set_role_and_error" statement 1
1368+
set debug_parallel_query = 1;
1369+
select set_and_report_role();
1370+
set_and_report_role
1371+
-------------------------
1372+
regress_parallel_worker
1373+
(1 row)
1374+
1375+
select set_role_and_error(0);
1376+
ERROR: division by zero
1377+
CONTEXT: SQL function "set_role_and_error" statement 1
1378+
parallel worker
1379+
reset debug_parallel_query;
1380+
drop function set_and_report_role();
1381+
drop function set_role_and_error(int);
1382+
drop role regress_parallel_worker;

‎src/test/regress/sql/select_parallel.sql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,3 +520,26 @@ select current_setting('session_authorization');
520520
set debug_parallel_query=1;
521521
select current_setting('session_authorization');
522522
rollback;
523+
524+
-- test that function option SET ROLE works in parallel workers.
525+
create role regress_parallel_worker;
526+
527+
createfunctionset_and_report_role() returnstextas
528+
$$select current_setting('role') $$ language sql parallel safe
529+
set role= regress_parallel_worker;
530+
531+
createfunctionset_role_and_error(int) returnsintas
532+
$$select1/ $1 $$ language sql parallel safe
533+
set role= regress_parallel_worker;
534+
535+
set debug_parallel_query=0;
536+
select set_and_report_role();
537+
select set_role_and_error(0);
538+
set debug_parallel_query=1;
539+
select set_and_report_role();
540+
select set_role_and_error(0);
541+
reset debug_parallel_query;
542+
543+
dropfunction set_and_report_role();
544+
dropfunction set_role_and_error(int);
545+
drop role regress_parallel_worker;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp