I sat down to write the two PUBLIC cases I suggested, and hit something bigger
on the way. As it stands the test does not distinguish the fixed pg_dump from
the broken one.
I took v7, reverted just the two SAFE_INITPRIVS call sites in pg_dump.c, left
the rest in place, rebuilt, and re-ran the test as posted -- with nothing added
but the quote_ident() from my last mail, so that it runs at all:
t/008_pg_dump_dangling_initprivs.pl .. ok
All tests successful.
Files=1, Tests=12
The objects that carry the dangling entries all keep their default ACL, so
pg_dump emits no GRANT/REVOKE for them at all, patched or not, and the
assertions that matter are all unlike() -- they hold vacuously.
--binary-upgrade doesn't change that.
One added line per case is enough to make it bite: give the object a non-default
ACL. For case 1,
CREATE FUNCTION public.test_func_grantee() RETURNS int LANGUAGE
sql AS 'SELECT 1';
REVOKE ALL ON FUNCTION public.test_func_grantee() FROM PUBLIC;
and case 1 then fails on the reverted build and passes on v7. The dump from the
reverted build is the reported breakage:
REVOKE ALL ON FUNCTION public.test_func_grantee() FROM "16385";
GRANT ALL ON FUNCTION public.test_func_grantee() TO "rui.zhao";
against v7's:
REVOKE ALL ON FUNCTION public.test_func_grantee() FROM PUBLIC;
That also shows the catch-all assertion is too narrow: it looks for
GRANT ... TO "digits", but the bare OID here lands in a
REVOKE ... FROM "digits".
Built the same way, the two PUBLIC cases from my last mail do bite as well: a
PUBLIC entry with a dangling grantor fails on the reverted build, and a valid
PUBLIC entry stops being dumped if the "ace.grantee <> 0" guard is removed.
Attached is a test-only patch on top of v7 that does all of the above:
quote_ident() on the four literals, a non-default ACL for the objects in cases
1 to 3, the widened catch-all, and the two PUBLIC cases. With that patch in
place the same three builds give:
v7's pg_dump as posted 14/14 pass
without the SAFE_INITPRIVS calls cases 1, 2, 3, 6 and the catch-all fail
without "ace.grantee <> 0" case 7 fails
One I left alone: case 4, the spurious-REVOKE one. Its premise is that proacl is
NULL, and that is exactly what stops pg_dump emitting anything for the object,
so I could not give it a non-default ACL the way I did for the others without
turning it into case 1. I did try to reproduce the spurious REVOKE with proacl
NULL a few other ways -- on a plain function, on a pinned catalog function, and
on a real extension member, plain and with --binary-upgrade -- and got no ACL
output at all in any of them. Where a bare OID does turn up in a REVOKE is when
the object has a non-default ACL, which cases 1 to 3 now cover. So the
precondition in that part of the commit message may not be quite right; you'd
know better whether there's a path I'm missing.
Thanks,
Rui
