Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table

Hi Hüseyin,

v8 covers both of my v7 points.

Four things I'd raise.

1. Add the test to src/bin/pg_dump/meson.build. That file lists the TAP tests
one at a time and this one is not in it, so only make runs it (prove_check
globs t/*.pl) -- "meson test --list | grep -c dangling" prints 0, so CI never
runs it at all.

2. Case 9 looks for a numeric OID in a GRANT or a REVOKE, and a dangling
grantor never lands there: it lands in the SET SESSION AUTHORIZATION that
dumpACL() wraps the initial-privileges block in, which is the command the
reporter's restore died on. With only the grantor half of SAFE_INITPRIVS
removed, case 9 passes on a dump containing

SET SESSION AUTHORIZATION "16386";
GRANT ALL ON FUNCTION public.test_func_public_ghost() TO PUBLIC;

Case 9 does not catch the first of those two lines, which is the one that
matters.

3. Say in the commit message that when every item of an object is dangling,
its pg_init_privs row goes away and does not come back. I had an extension
whose script does

REVOKE ALL ON FUNCTION orphan_reset() FROM PUBLIC;
GRANT SELECT ON orphan_view TO PUBLIC;

installed by a role that was then dropped, leaving

objoid | classoid | privtype | initprivs
--------+----------+----------+---------------------------------
16387 | pg_proc | e | {16384=X/16384}
16388 | pg_class | e | {16384=arwdDxtm/16384,=r/16384}

After the upgrade the new cluster has no pg_init_privs row for either, the
ACLs are right, and a plain pg_dump of it emits

CREATE EXTENSION IF NOT EXISTS orphantest WITH SCHEMA public;
REVOKE ALL ON FUNCTION public.orphan_reset() FROM PUBLIC;
GRANT SELECT ON TABLE public.orphan_view TO PUBLIC;

where the same database with the extension freshly installed emits the
CREATE EXTENSION line alone: with no initprivs to diff against, pg_dump falls
back to the type default and re-emits what the extension script already does.
Harmless on restore, but permanent, and it makes the members look like
someone changed their privileges.

4. Reconsider "Backpatch-through: 14". getAdditionalACLs() does not exist
before v15 (0c9d84427f); v14's pg_dump LEFT JOINs pg_init_privs in a dozen
per-object queries instead. What makes the filter cheap here is that
getAdditionalACLs() runs once per dump; v14 has no such place, so
SAFE_INITPRIVS would land in queries returning a row per object -- the v4
shape, and what drew the performance objection. v15 and up should take this
nearly as-is. And since pg_upgrade runs the new cluster's pg_dump, the
reported v14 -> v18 case is fixed by the v18 copy alone; a v14 backpatch only
helps pg_dump run out of a v14 installation.

The attached test-only patch does 1 and 2. make check passes in
src/bin/pg_dump and src/bin/pg_upgrade, and under meson once the test is
registered.

Thanks,
Rui