From: Tom Lane <tgl@sss.pgh.pa.us>
To: Laurenz Albe <laurenz.albe@cybertec.at>
cc: =?ISO-8859-1?Q?H=FCseyin?= Demir <huseyin.d3r@gmail.com>,
        Greg Sabino Mullane <htamfids@gmail.com>,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv
 catalog table
In-reply-to: <0d7531c192859057c6aad54c89a8d453376bd00c.camel@cybertec.at>
References: <19483-80de42dc4e62cfd6@postgresql.org>
 <CAKAnmmLYXcBSV8i5MVuiwu8tkX-JDGmZtJ38_YbdUTwoDW7xKg@mail.gmail.com>
 <104812.1780847519@sss.pgh.pa.us>
 <CAB5wL7bo-7okBCJ24fdVn4UhnuPLPG22eQjaLJZ9GJtv0FEWMg@mail.gmail.com>
 <CAB5wL7bunPCi93Bwi7zN_1gFAZhKLwS=Fsub91SW417xujGtNA@mail.gmail.com>
 <CAB5wL7aKGFFEF1SM54GxvaQPH=7Bs=HuUCqi7F4Ve025+Z950A@mail.gmail.com>
 <CAB5wL7aK6UaukMSegzEG68y0VrKOg7wFnPNRaDaHreBf2iVWag@mail.gmail.com>
 <d305af372949be0c2c4155168ca7ba97e160bc52.camel@cybertec.at>
 <5184088806e70ed5db7e83bd6540a016b3802c1d.camel@cybertec.at>
 <CAB5wL7ZuFwu2Sd-xmn-w5-BbrKk_Mu_1ubYow_qaqFGhJOiMXw@mail.gmail.com>
 <CAB5wL7ai8AiaFV6B8J=w6vK1Q6Z9Hmp_y3PFL2Zzpx6Y8xPZ2A@mail.gmail.com>
 <0d7531c192859057c6aad54c89a8d453376bd00c.camel@cybertec.at>
Comments: In-reply-to Laurenz Albe <laurenz.albe@cybertec.at>
	message dated "Mon, 22 Jun 2026 19:23:38 +0200"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <592791.1782152994.0@sss.pgh.pa.us>
Date: Mon, 22 Jun 2026 14:30:54 -0400
Message-ID: <592839.1782153054@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/592839.1782153054%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <592791.1782152994.1@sss.pgh.pa.us>

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> Since there have been very few reports of this problem, the question
> remains if we need this patch at all, or of it should be backpatched.
> My opinion is that it should; every upgrade or restore failure is
> one too many.

I have a more pressing concern: has any performance testing been
done on this?  It looks like it'd be absolutely catastrophic for
pg_dump performance on databases with lots of objects.

The implementation direction I'd been vaguely imagining was for
pg_dump's buildACLCommands() to drop any AclItems that contain
dangling role references (ie, numeric OIDs where a role name
should be).  If the given role name contains any non-digit
characters then it's certainly not dangling, so most of the time
this'd be a very cheap check.  However, if somebody does

	CREATE USER "007";
	GRANT ALL ON TABLE mi6_operations TO "007";

we mustn't get fooled by that.  The backend is doing us no favors by
not making numeric OIDs visibly different from all-digit role names
in AclItems.  In HEAD I'd advocate fixing that on the server side
(as attached), but we can't assume that a back-branch server has such
a fix.  What we could do with an old server is issue a query (once per
pg_dump run) to collect all the valid all-digit role names, which
should surely be a short list in most databases, and then filter
against that within buildACLCommands().

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="quote-all-digit-role-names-in-aclitemout.patch";
	charset="us-ascii"
Content-ID: <592791.1782152994.2@sss.pgh.pa.us>
Content-Description: quote-all-digit-role-names-in-aclitemout.patch

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 01caa12eca7..84803351f18 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -235,6 +235,10 @@ putid(char *p, const char *s)
 			break;
 		}
 	}
+	/* If name is all-digits, quote it to distinguish from lookup failure */
+	if (safe && strspn(s, "0123456789") == strlen(s))
+		safe = false;
+
 	if (!safe)
 		*p++ = '"';
 	for (src = s; *src; src++)

------- =_aaaaaaaaaa0--