mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-11-21 03:13:05 +08:00
Allow parallel workers to cope with a newly-created session user ID.
Parallel workers failed after a sequence like BEGIN; CREATE USER foo; SET SESSION AUTHORIZATION foo; because check_session_authorization could not see the uncommitted pg_authid row for "foo". This is because we ran RestoreGUCState() in a separate transaction using an ordinary just-created snapshot. The same disease afflicts any other GUC that requires catalog lookups and isn't forgiving about the lookups failing. To fix, postpone RestoreGUCState() into the worker's main transaction after we've set up a snapshot duplicating the leader's. This affects check_transaction_isolation and check_transaction_deferrable, which think they should only run during transaction start. Make them act like check_transaction_read_only, which already knows it should silently accept the value when InitializingParallelWorker. This un-reverts commitf5f30c22e
. The original plan was to back-patch that, but the fact that0ae5b763e
proved to be a pre-requisite shows that the subtle API change for GUC hooks might actually break some of them. The problem we're trying to fix seems not worth taking such a risk for in stable branches. Per bug #18545 from Andrey Rachitskiy. Discussion: https://postgr.es/m/18545-feba138862f19aaa@postgresql.org
This commit is contained in:
parent
0ae5b763ea
commit
6e086fa2e7
@ -1414,10 +1414,6 @@ ParallelWorkerMain(Datum main_arg)
|
||||
libraryspace = shm_toc_lookup(toc, PARALLEL_KEY_LIBRARY, false);
|
||||
StartTransactionCommand();
|
||||
RestoreLibraryState(libraryspace);
|
||||
|
||||
/* Restore GUC values from launching backend. */
|
||||
gucspace = shm_toc_lookup(toc, PARALLEL_KEY_GUC, false);
|
||||
RestoreGUCState(gucspace);
|
||||
CommitTransactionCommand();
|
||||
|
||||
/* Crank up a transaction state appropriate to a parallel worker. */
|
||||
@ -1459,6 +1455,14 @@ ParallelWorkerMain(Datum main_arg)
|
||||
*/
|
||||
InvalidateSystemCaches();
|
||||
|
||||
/*
|
||||
* Restore GUC values from launching backend. We can't do this earlier,
|
||||
* because GUC check hooks that do catalog lookups need to see the same
|
||||
* database state as the leader.
|
||||
*/
|
||||
gucspace = shm_toc_lookup(toc, PARALLEL_KEY_GUC, false);
|
||||
RestoreGUCState(gucspace);
|
||||
|
||||
/*
|
||||
* Restore current role id. Skip verifying whether session user is
|
||||
* allowed to become this role and blindly restore the leader's state for
|
||||
|
@ -577,14 +577,16 @@ check_transaction_read_only(bool *newval, void **extra, GucSource source)
|
||||
* We allow idempotent changes at any time, but otherwise this can only be
|
||||
* changed in a toplevel transaction that has not yet taken a snapshot.
|
||||
*
|
||||
* As in check_transaction_read_only, allow it if not inside a transaction.
|
||||
* As in check_transaction_read_only, allow it if not inside a transaction,
|
||||
* or if restoring state in a parallel worker.
|
||||
*/
|
||||
bool
|
||||
check_transaction_isolation(int *newval, void **extra, GucSource source)
|
||||
{
|
||||
int newXactIsoLevel = *newval;
|
||||
|
||||
if (newXactIsoLevel != XactIsoLevel && IsTransactionState())
|
||||
if (newXactIsoLevel != XactIsoLevel &&
|
||||
IsTransactionState() && !InitializingParallelWorker)
|
||||
{
|
||||
if (FirstSnapshotSet)
|
||||
{
|
||||
@ -619,6 +621,10 @@ check_transaction_isolation(int *newval, void **extra, GucSource source)
|
||||
bool
|
||||
check_transaction_deferrable(bool *newval, void **extra, GucSource source)
|
||||
{
|
||||
/* Just accept the value when restoring state in a parallel worker */
|
||||
if (InitializingParallelWorker)
|
||||
return true;
|
||||
|
||||
if (IsSubTransaction())
|
||||
{
|
||||
GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION);
|
||||
|
@ -1329,3 +1329,21 @@ SELECT 1 FROM tenk1_vw_sec
|
||||
(9 rows)
|
||||
|
||||
rollback;
|
||||
-- test that a newly-created session role propagates to workers.
|
||||
begin;
|
||||
create role regress_parallel_worker;
|
||||
set session authorization regress_parallel_worker;
|
||||
select current_setting('session_authorization');
|
||||
current_setting
|
||||
-------------------------
|
||||
regress_parallel_worker
|
||||
(1 row)
|
||||
|
||||
set debug_parallel_query = 1;
|
||||
select current_setting('session_authorization');
|
||||
current_setting
|
||||
-------------------------
|
||||
regress_parallel_worker
|
||||
(1 row)
|
||||
|
||||
rollback;
|
||||
|
@ -511,3 +511,12 @@ SELECT 1 FROM tenk1_vw_sec
|
||||
WHERE (SELECT sum(f1) FROM int4_tbl WHERE f1 < unique1) < 100;
|
||||
|
||||
rollback;
|
||||
|
||||
-- test that a newly-created session role propagates to workers.
|
||||
begin;
|
||||
create role regress_parallel_worker;
|
||||
set session authorization regress_parallel_worker;
|
||||
select current_setting('session_authorization');
|
||||
set debug_parallel_query = 1;
|
||||
select current_setting('session_authorization');
|
||||
rollback;
|
||||
|
Loading…
Reference in New Issue
Block a user