Re: Major Version Upgrade failure due to orphan roles entries in catalog

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Virender Singla <virender(dot)cse(at)gmail(dot)com>, pgsql-bugs(at)lists(dot)postgresql(dot)org, Aniket Jha <aniketkumarj(at)gmail(dot)com>
Subject: Re: Major Version Upgrade failure due to orphan roles entries in catalog
Date: 2025-02-21 07:18:12
Message-ID: b0459688caa95c77f439bebf3b30341169986369.camel@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Thu, 2025-02-20 at 17:19 -0500, Tom Lane wrote:
> After looking at this I thought it was worth a little more code to warn
> about the dangling role OID, instead of just silently ignoring it.
> Here's a couple of more-polished patches.
>
> I'm unsure whether to back-patch the 0001 patch, as it does imply
> more pg_shdepend entries than we have today, so it's sort of a
> backdoor catalog change. But we're mostly interested in the
> transient behavior of having a lock+recheck during entry insertion,
> so maybe it's fine. 0002 should be back-patched in any case.

I'd say that adding new catalog entries in a way that is compatible
shouldn't be a problem, but I still wouldn't backpatch the 0001 patch,
because it is not necessary. The orphaned pg_auth_members entry didn't
cause any harm, and a few warnings more during an upgrade shouldn't be
a big problem.

I have one question about the first patch:

> diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
> index 0db174e6f1..0c84886e82 100644
> --- a/src/backend/commands/user.c
> +++ b/src/backend/commands/user.c
> @@ -489,7 +490,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
> * Advance command counter so we can see new record; else tests in
> * AddRoleMems may fail.
> */
> - if (addroleto || adminmembers || rolemembers)
> + if (addroleto || adminmembers || rolemembers || !superuser())
> CommandCounterIncrement();
>
> /* Default grant. */

That change seems unrelated to the problem at hand, and I don't see it
mentioned in the commit message. Is that an oversight you fixed on the
fly?

Apart from that, the patches look fine.

Yours,
Laurenz Albe

--

*E-Mail Disclaimer*
Der Inhalt dieser E-Mail ist ausschliesslich fuer den
bezeichneten Adressaten bestimmt. Wenn Sie nicht der vorgesehene Adressat
dieser E-Mail oder dessen Vertreter sein sollten, so beachten Sie bitte,
dass jede Form der Kenntnisnahme, Veroeffentlichung, Vervielfaeltigung oder
Weitergabe des Inhalts dieser E-Mail unzulaessig ist. Wir bitten Sie, sich
in diesem Fall mit dem Absender der E-Mail in Verbindung zu setzen.

*CONFIDENTIALITY NOTICE & DISCLAIMER
*This message and any attachment are
confidential and may be privileged or otherwise protected from disclosure
and solely for the use of the person(s) or entity to whom it is intended.
If you have received this message in error and are not the intended
recipient, please notify the sender immediately and delete this message and
any attachment from your system. If you are not the intended recipient, be
advised that any use of this message is prohibited and may be unlawful, and
you must not copy this message or attachment or disclose the contents to
any other person.

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Jelte Fennema-Nio 2025-02-21 09:23:57 Re: Error in form on site commitfest.postgresql.org
Previous Message Vladlen Popolitov 2025-02-21 04:57:54 Re: Error in form on site commitfest.postgresql.org