Re: catalog access with reset GUCs during parallel worker startup

On Wed, 23 Feb 2022 at 15:51, Andres Freund <andres@anarazel.de> wrote:
> The tests fail:
> +ERROR:  invalid value for parameter "session_authorization": "andres"
> +CONTEXT:  while setting parameter "session_authorization" to "andres"
> +parallel worker
> +while scanning relation "public.pvactst"
>
> but that's easily worked around. In fact, there's already code to do so, it
> just is lacking awareness of session_authorization:

I was experimenting with the attached patch which just has
check_session_authorization() return true when not in transaction and
when InitializingParallelWorker is true. It looks like there are
already some other check functions looking at the value of
InitializingParallelWorker (e.g. check_transaction_read_only(),
check_role())

With that done, I do not see any other errors when calling
RestoreGUCState() while not in transaction.  The parallel worker seems
to pick up the session_authorization GUC correctly using the same
debug_parallel_query test that you tried out.

On looking at the other check functions, I noted down the following as
ones that may behave differently when called while not in transaction:

check_transaction_read_only --- checks InitializingParallelWorker
check_transaction_deferrable -- checks
IsSubTransaction/FirstSnapshotSet, returns true if not
check_client_encoding -- Checks IsTransactionState().  Behavioural change
check_default_table_access_method -- accesses catalog if in xact.
Behavioural change
check_default_tablespace -- accesses catalog if in xact. Behavioural change
check_temp_tablespaces -- accesses catalog if in xact. Behavioural change
check_role -- Returns false if not in xact. Handled specially in
RestoreGUCState()
check_session_authorization -- Modify by patch to return true when not
in xact and parallel worker starting
check_timezone -- calls interval_in. Can't see any catalog lookup there.
check_default_text_search_config -- Checks in IsTransactionState()
check_transaction_isolation -- Check in IsTransactionState()

It seems that RestoreLibraryState() was called while in transaction so
that extension GUCs could be initialized before restoring the GUCs.
So there could be repercussions for extensions if we did this.

Does anyone see any reasons why we can't fix this with the attached?

David