From: Андрей Рачицкий <therealgofman@mail.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>
Subject: Re: BUG #18545: \dt breaks transaction, calling error when executed in SET SESSION AUTHORIZATION
Date: Mon, 29 Jul 2024 11:24:48 +0300
Message-ID: <1722241488.182053205@f169.i.mail.ru> (raw)
In-Reply-To: <72802.1721415837@sss.pgh.pa.us>
References: <18545-feba138862f19aaa@postgresql.org>
<72802.1721415837@sss.pgh.pa.us>
Hi, Tom! Thank you for work on the subject. After applying patch, problem is no longer reproducible.
---
Best regards,
Andrey Rachitskiy
Postgres Professional: http://postgrespro.com>Суббота, 20 июля 2024, 0:04 +05:00 от Tom Lane <tgl@sss.pgh.pa.us>:
>
>PG Bug reporting form < noreply@postgresql.org > writes:
>> postgres=# BEGIN;
>> BEGIN
>> postgres=*# CREATE USER regress_priv_user8;
>> CREATE ROLE
>> postgres=*# SET SESSION AUTHORIZATION regress_priv_user8;
>> SET
>> postgres=*> SET LOCAL debug_parallel_query = 1;
>> SET
>> postgres=*> \dt+;
>> ERROR: 22023: role "regress_priv_user8" does not exist
>> CONTEXT: while setting parameter "session_authorization" to "regress_priv_user8"
>> parallel worker
>So this has exactly nothing to do with \dt+; any parallel query
>will hit it. The problem is that parallel workers do
>RestoreGUCState() before they've restored the leader's snapshot.
>Thus, in this example where session_authorization refers to an
>uncommitted pg_authid entry, the workers don't see that entry.
>It seems likely that similar failures are possible with other
>GUCs that perform catalog lookups.
>
>I experimented with two different ways to fix this:
>
>1. Run RestoreGUCState() outside a transaction, thus preventing
>catalog lookups. Assume that individual GUC check hooks that
>would wish to do a catalog lookup will cope. Unfortunately,
>some of them don't and would need fixed; check_role and
>check_session_authorization for two.
>
>2. Delay RestoreGUCState() into the parallel worker's main
>transaction, after we've restored the leader's snapshot.
>This turns out to break a different set of check hooks, notably
>check_transaction_deferrable.
>
>I think that the blast radius of option 2 is probably smaller than
>option 1's, because it should only matter to check hooks that think
>they should run before the transaction has set a snapshot, and there
>are few of those. check_transaction_read_only already had a guard,
>but I added similar ones to check_transaction_isolation and
>check_transaction_deferrable.
>
>The attached draft patch also contains changes to prevent
>check_session_authorization from doing anything during parallel
>worker startup. That's left over from experimenting with option 1,
>and is not strictly necessary with option 2. I left it in anyway
>because it's saving some unnecessary work. (For some reason,
>check_role seems not to fail if you modify the test case to use
>SET ROLE. I did not figure out why not. I kind of want to modify
>check_role to be a no-op too when InitializingParallelWorker,
>but did not touch that here pending more investigation.)
>
>Another thing I'm wondering about is whether to postpone
>RestoreLibraryState similarly. Its current placement is said
>to be "before restoring GUC values", so it looks a little out
>of place now. Moving it into the main transaction would save
>one StartTransactionCommand/CommitTransactionCommand pair
>during parallel worker start, which is worth something.
>But I think the real argument for it is that if any loaded
>libraries try to do catalog lookups during load, we'd rather
>that they see the same catalog state the leader does.
>As against that, it feels like there's a nonzero risk of
>breaking some third-party code if we move that call.
>
>Thoughts?
>
>regards, tom lane
>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: therealgofman@mail.ru, tgl@sss.pgh.pa.us, robertmhaas@gmail.com
Subject: Re: Re: BUG #18545: \dt breaks transaction, calling error when executed in SET SESSION AUTHORIZATION
In-Reply-To: <1722241488.182053205@f169.i.mail.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox