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
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++)
