Re: pg_upgrade fails with non-standard ACL

On Thu, Jan 21, 2021 at 01:03:58AM +0300, Anastasia Lubennikova wrote:
> On 03.01.2021 14:29, Noah Misch wrote:
> >On Thu, Jun 11, 2020 at 07:58:43PM +0300, Anastasia Lubennikova wrote:

> Thank you for the review.
> New version of the patch is attached, though I haven't tested it properly
> yet. I am planning to do in a couple of days.

Once that testing completes, please change the commitfest entry status to
Ready for Committer.

> >>+ snprintf(output_path, sizeof(output_path), "revoke_objects.sql");
> >If an upgrade deleted function pg_last_wal_replay_lsn, upgrading a database in
> >which "REVOKE ALL ON FUNCTION pg_last_wal_replay_lsn FROM public" happened
> >requires a GRANT. Can you use a file name that will still make sense if
> >someone enhances pg_upgrade to generate such GRANT statements?
> I changed the name to 'fix_system_objects_ACL.sql'. Does it look good to
> you?

That name is fine with me.

> > ALTER VIEW pg_locks RENAME COLUMN objid TO "a.b";
> > GRANT SELECT ("a.b") ON pg_locks TO backup;
> >
> >After which revoke_objects.sql has:
> >
> > psql:./revoke_objects.sql:9: ERROR: syntax error at or near "") ON pg_catalog.pg_locks.""
> > LINE 1: REVOKE ALL (b") ON pg_catalog.pg_locks."a FROM "backup";
> >
> >While that ALTER VIEW probably should have required allow_system_table_mods,
> >we need to assume such databases exist.
>
> I've fixed it by using pg_identify_object_as_address(). Now we don't have to
> parse obj_identity.

Nice.