public inbox for [email protected]  

help / color / mirror / Atom feed

From: Robert Haas <[email protected]>
To: Tom Lane <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: Virender Singla <[email protected]>
Cc: [email protected]
Cc: Aniket Jha <[email protected]>
Subject: Re: Major Version Upgrade failure due to orphan roles entries in catalog
Date: Wed, 25 Feb 2026 08:59:04 -0500
Message-ID: <CA+TgmoauoiW4ydDhdrseg+DD4Kwha=+TSZp18BrJeHKx3o1Fdw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

On Thu, Feb 20, 2025 at 5:19 PM Tom Lane <[email protected]> wrote:
> 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 recently learned of a case where this commit caused role grants to
be erroneously emitted from the output of pg_dumpall. In the case in
question, a v16 pg_dumpall was used against an older server.  Hence,
dump_grantors was false, and any generated GRANT commands would not
have included in the grantor anyway. Nevertheless, this logic caused
those grants to be skipped altogether:

+                if (PQgetisnull(res, i, i_grantor))
+                {
+                    /* translator: %s represents a numeric role OID */
+                    pg_log_warning("found orphaned pg_auth_members
entry for role %s",
+                                   PQgetvalue(res, i, i_grantorid));
+                    done[i - start] = true;
+                    --remaining;
+                    continue;
+                }

I don't think this logic makes sense. In pre-16 releases, we don't
even try to maintain the grantor field properly. Consider this test
case:

create role foo;
create role bar;
create role baz createrole;
set role baz;
grant foo to bar;
reset role;
drop role baz;

If you do this on v15 and then run v15's pg_dumpall, it will dump
"GRANT foo to bar", with no GRANTOR clause due to the PQgetisnull()
gating that logic. v16's pg_dumpall will dump nothing and emit a
warning instead. Arguably, pre-v16 pg_dumpall shouldn't EVER be
dumping the grantor since the grantorid could be an old role OID that
has been recycled for a new role, and relying on that for anything
security-critical seems like a mistake, but that behavior is also
longstanding. But omitting the grant altogether seems like an
overreaction. I understand that we need to do that when the *member*
is invalid, of course; in that case, there's no alternative. But
that's not the case for the grantor.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Major Version Upgrade failure due to orphan roles entries in catalog
  In-Reply-To: <CA+TgmoauoiW4ydDhdrseg+DD4Kwha=+TSZp18BrJeHKx3o1Fdw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox