From a4649c50a8db58c19fa9a4f2ae12bef52b6c872b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 2 Mar 2026 11:14:58 -0500 Subject: [PATCH] In pg_dumpall, don't skip role GRANTs with dangling grantor OIDs. In commits 29d75b25b et al, I made pg_dumpall's dumpRoleMembership logic treat a dangling grantor OID the same as dangling role and member OIDs: print a warning and skip emitting the GRANT. This wasn't terribly well thought out; instead, we should handle the case by emitting the GRANT without the GRANTED BY clause. When the source database is pre-v16, such cases are somewhat expected because those versions didn't prevent dropping the grantor role; so don't even print a warning that we did this. (This change therefore restores pg_dumpall's pre-v16 behavior for these cases.) The case is not expected in >= v16, so then we do print a warning, but soldiering on with no GRANTED BY clause still seems like a reasonable strategy. Per complaint from Robert Haas that we were now dropping GRANTs altogether in easily-reachable scenarios. Reported-by: Robert Haas Author: Tom Lane Discussion: https://postgr.es/m/CA+TgmoauoiW4ydDhdrseg+DD4Kwha=+TSZp18BrJeHKx3o1Fdw@mail.gmail.com Backpatch-through: 16 --- src/bin/pg_dump/pg_dumpall.c | 49 ++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index b534df14b9e..84dde319467 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -1017,7 +1017,7 @@ dumpRoleMembership(PGconn *conn) * that no longer exist. If we find such cases, print a warning and skip * the entry. */ - dump_grantors = (PQserverVersion(conn) >= 160000); + dump_grantors = (server_version >= 160000); /* * Previous versions of PostgreSQL also did not have grant-level options. @@ -1082,7 +1082,7 @@ dumpRoleMembership(PGconn *conn) if (PQgetisnull(res, start, i_role)) { /* translator: %s represents a numeric role OID */ - pg_log_warning("found orphaned pg_auth_members entry for role %s", + pg_log_warning("ignoring role grant for missing role with OID %s", PQgetvalue(res, start, i_roleid)); break; } @@ -1099,6 +1099,11 @@ dumpRoleMembership(PGconn *conn) remaining = end - start; done = pg_malloc0(remaining * sizeof(bool)); + + /* + * We use a hashtable to track the member names that have been granted + * admin option. Usually a hashtable is overkill, but sometimes not. + */ ht = rolename_create(remaining, NULL); /* @@ -1126,50 +1131,56 @@ dumpRoleMembership(PGconn *conn) for (i = start; i < end; ++i) { char *member; - char *admin_option; char *grantorid; - char *grantor; + char *grantor = NULL; + bool dump_this_grantor = dump_grantors; char *set_option = "true"; + char *admin_option; bool found; /* If we already did this grant, don't do it again. */ if (done[i - start]) continue; - /* Complain about, then ignore, entries with orphaned OIDs. */ + /* Complain about, then ignore, entries for unknown members. */ if (PQgetisnull(res, i, i_member)) { /* translator: %s represents a numeric role OID */ - pg_log_warning("found orphaned pg_auth_members entry for role %s", + pg_log_warning("ignoring role grant to missing role with OID %s", PQgetvalue(res, i, i_memberid)); done[i - start] = true; --remaining; continue; } - if (PQgetisnull(res, i, i_grantor)) + member = PQgetvalue(res, i, i_member); + + /* If the grantor is unknown, complain and dump without it. */ + grantorid = PQgetvalue(res, i, i_grantorid); + if (dump_this_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; + if (PQgetisnull(res, i, i_grantor)) + { + /* translator: %s represents a numeric role OID */ + pg_log_warning("grant of role \"%s\" to \"%s\" has invalid grantor OID %s", + role, member, grantorid); + pg_log_warning_detail("This grant will be dumped without GRANTED BY."); + dump_this_grantor = false; + } + else + grantor = PQgetvalue(res, i, i_grantor); } - member = PQgetvalue(res, i, i_member); - grantor = PQgetvalue(res, i, i_grantor); - grantorid = PQgetvalue(res, i, i_grantorid); admin_option = PQgetvalue(res, i, i_admin_option); if (dump_grant_options) set_option = PQgetvalue(res, i, i_set_option); /* - * If we're not dumping grantors or if the grantor is the + * If we're not dumping the grantor or if the grantor is the * bootstrap superuser, it's fine to dump this now. Otherwise, * it's got to be someone who has already been granted ADMIN * OPTION. */ - if (dump_grantors && + if (dump_this_grantor && atooid(grantorid) != BOOTSTRAP_SUPERUSERID && rolename_lookup(ht, grantor) == NULL) continue; @@ -1210,7 +1221,7 @@ dumpRoleMembership(PGconn *conn) } if (optbuf->data[0] != '\0') fprintf(OPF, " WITH %s", optbuf->data); - if (dump_grantors) + if (dump_this_grantor) fprintf(OPF, " GRANTED BY %s", fmtId(grantor)); fprintf(OPF, ";\n"); }