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 commit f5f30c22e.  The original plan was to back-patch
that, but the fact that 0ae5b763e 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

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 27551566ac421933ea0b6d35cd0ecd30aae695a7..9aba17bd5e9d7775b8bccda2c4c6140c8b0d3e8e 100644 (file)
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -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

diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 6ba6d08b24123df2ed8254f89072d31fe13706fe..6202c5ebe44b3d4476dc2f1a1d1bc4bebb5c4e94 100644 (file)
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -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);

diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index 5a603f86b7366dbacd18080f8df658f4fcb04a51..68a629619a6eb4bd4725a6bfb1c485863c93e1e1 100644 (file)
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -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;

diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index c7df8f775cee90d6975a27239aa2847468cedb32..1a0e7f144d02e90e626296c27eda1e678a409b44 100644 (file)
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -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;