Fix pg_dumpall to cope with dangling OIDs in pg_auth_members.

There is a race condition between "GRANT role" and "DROP ROLE",
which allows GRANT to install pg_auth_members entries that refer to
dropped roles.  (Commit 6566133c5 prevented that for the grantor
field, but not for the granted or grantee roles.)  We'll soon fix
that, at least in HEAD, but pg_dumpall needs to cope with the
situation in case of pre-existing inconsistency.  As pg_dumpall
stands, it will emit invalid commands like 'GRANT foo TO ""',
which causes pg_upgrade to fail.  Fix it to emit warnings and skip
those GRANTs, instead.

There was some discussion of removing the problem by changing
dumpRoleMembership's query to use JOIN not LEFT JOIN, but that
would result in silently ignoring such entries.  It seems better
to produce a warning.

Pre-v16 branches already coped with dangling grantor OIDs by simply
omitting the GRANTED BY clause.  I left that behavior as-is, although
it's somewhat inconsistent with the behavior of later branches.

Reported-by: Virender Singla <virender.cse@gmail.com>
Discussion: https://postgr.es/m/CAM6Zo8woa62ZFHtMKox6a4jb8qQ=w87R2L0K8347iE-juQL2EA@mail.gmail.com
Backpatch-through: 13

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 9d8732ac736673536c67b5c7f949c5eec33b6b9b..36408b3676c87767360b24a4a95066dd4d4e20f9 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -960,6 +960,13 @@ dumpRoleMembership(PGconn *conn)
                total;
    bool        dump_grantors;
    bool        dump_grant_options;
+   int         i_role;
+   int         i_member;
+   int         i_grantor;
+   int         i_roleid;
+   int         i_memberid;
+   int         i_grantorid;
+   int         i_admin_option;
    int         i_inherit_option;
    int         i_set_option;
 
@@ -969,6 +976,10 @@ dumpRoleMembership(PGconn *conn)
     * they didn't have ADMIN OPTION on the role, or a user that no longer
     * existed. To avoid dump and restore failures, don't dump the grantor
     * when talking to an old server version.
+    *
+    * Also, in older versions the roleid and/or member could be role OIDs
+    * that no longer exist.  If we find such cases, print a warning and skip
+    * the entry.
     */
    dump_grantors = (PQserverVersion(conn) >= 160000);
 
@@ -980,8 +991,10 @@ dumpRoleMembership(PGconn *conn)
    /* Generate and execute query. */
    printfPQExpBuffer(buf, "SELECT ur.rolname AS role, "
                      "um.rolname AS member, "
-                     "ug.oid AS grantorid, "
                      "ug.rolname AS grantor, "
+                     "a.roleid AS roleid, "
+                     "a.member AS memberid, "
+                     "a.grantor AS grantorid, "
                      "a.admin_option");
    if (dump_grant_options)
        appendPQExpBufferStr(buf, ", a.inherit_option, a.set_option");
@@ -990,8 +1003,15 @@ dumpRoleMembership(PGconn *conn)
                      "LEFT JOIN %s um on um.oid = a.member "
                      "LEFT JOIN %s ug on ug.oid = a.grantor "
                      "WHERE NOT (ur.rolname ~ '^pg_' AND um.rolname ~ '^pg_')"
-                     "ORDER BY 1,2,4", role_catalog, role_catalog, role_catalog);
+                     "ORDER BY 1,2,3", role_catalog, role_catalog, role_catalog);
    res = executeQuery(conn, buf->data);
+   i_role = PQfnumber(res, "role");
+   i_member = PQfnumber(res, "member");
+   i_grantor = PQfnumber(res, "grantor");
+   i_roleid = PQfnumber(res, "roleid");
+   i_memberid = PQfnumber(res, "memberid");
+   i_grantorid = PQfnumber(res, "grantorid");
+   i_admin_option = PQfnumber(res, "admin_option");
    i_inherit_option = PQfnumber(res, "inherit_option");
    i_set_option = PQfnumber(res, "set_option");
 
@@ -1015,24 +1035,32 @@ dumpRoleMembership(PGconn *conn)
    total = PQntuples(res);
    while (start < total)
    {
-       char       *role = PQgetvalue(res, start, 0);
+       char       *role = PQgetvalue(res, start, i_role);
        int         i;
        bool       *done;
        int         remaining;
        int         prev_remaining = 0;
        rolename_hash *ht;
 
+       /* If we hit a null roleid, we're done (nulls sort to the end). */
+       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",
+                          PQgetvalue(res, start, i_roleid));
+           break;
+       }
+
        /* All memberships for a single role should be adjacent. */
        for (end = start; end < total; ++end)
        {
            char       *otherrole;
 
-           otherrole = PQgetvalue(res, end, 0);
+           otherrole = PQgetvalue(res, end, i_role);
            if (strcmp(role, otherrole) != 0)
                break;
        }
 
-       role = PQgetvalue(res, start, 0);
        remaining = end - start;
        done = pg_malloc0(remaining * sizeof(bool));
        ht = rolename_create(remaining, NULL);
@@ -1072,10 +1100,30 @@ dumpRoleMembership(PGconn *conn)
                if (done[i - start])
                    continue;
 
-               member = PQgetvalue(res, i, 1);
-               grantorid = PQgetvalue(res, i, 2);
-               grantor = PQgetvalue(res, i, 3);
-               admin_option = PQgetvalue(res, i, 4);
+               /* Complain about, then ignore, entries with orphaned OIDs. */
+               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",
+                                  PQgetvalue(res, i, i_memberid));
+                   done[i - start] = true;
+                   --remaining;
+                   continue;
+               }
+               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;
+               }
+
+               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);