Re: pg_upgrade fails with non-standard ACL

On 28.01.2021 09:55, Noah Misch wrote:
> On Wed, Jan 27, 2021 at 07:32:42PM +0300, Anastasia Lubennikova wrote:
>> On 27.01.2021 14:21, Noah Misch wrote:
>>> On Mon, Jan 25, 2021 at 10:14:43PM +0300, Anastasia Lubennikova wrote:
>>>
>>>> 1) Could you please clarify, what do you mean by REVOKE failures?
>>>>
>>>> I tried following example:
>>>>
>>>> Start 9.6 cluster.
>>>>
>>>> REVOKE ALL ON function pg_switch_xlog() FROM public;
>>>> REVOKE ALL ON function pg_switch_xlog() FROM backup;
>>>>
>>>> The upgrade to the current master passes with and without patch.
>>>> It seems that current implementation of pg_upgrade doesn't take into account
>>>> revoke ACLs.
>>> I think you can observe it by adding "revoke all on function
>>> pg_stat_get_subscription from public;" to test_add_acl_to_catalog_objects.sql
>>> and then rerunning your test script.  pg_dump will reproduce that REVOKE,
>>> which would fail if postgresql.org had removed that function.  That's fine, so
>>> long as a comment mentions the limitation.
>> In the updated patch, I implemented generation of both GRANT ALL and REVOKE
>> ALL for problematic objects. If I understand it correctly, these calls will
>> clean object's ACL completely. And I see no harm in doing this, because the
>> objects don't exist in the new cluster anyway.
>>
>> To test it I attempted to reproduce the problem, using attached
>> test_revoke.sql in the test. Still, pg_upgrade works fine without any
>> adjustments. I'd be grateful if you test it some more.
> test_revoke.sql has "revoke all on function pg_stat_get_subscription() from
> test", which does nothing.  You need something that causes a REVOKE in pg_dump
> output.  Worked example:
> ...
It took a while to debug new version.
Now it detects and fixes both GRANT and REVOKE privileges on the catalog 
objects.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company