08.10.2019 17:08, Stephen Frost wrote:
>> I attached the updated version. Now it prints a better error message
>> and generates an SQL script instead of multiple warnings. The attached test
>> script shows that.
> Have you tested this with extensions, where the user has changed the
> privileges on the extension? I'm concerned that we'll throw out
> warnings and tell users to go REVOKE privileges on any case where the
> privileges on an extension's object were changed, but that shouldn't be
> necessary and we should be excluding those.
Good catch.
Fixed in v3.
I also added this check to previous test script. It passes with both v2
and v3,
but v3 doesn't throw unneeded warning for extension functions.
> Changes to privileges on extensions should be handled just fine using
> the existing code, at least for upgrades of PG.
>
> Otherwise, at least imv, the code could use more comments (inside the
> functions, not just function-level...) and there's a few wording
> improvements that could be made. Again, not a full endorsement, as I
> just took a quick look, but it generally seems like a reasonable
> approach to go in and the issue with extensions was the only thing that
> came to mind as a potential problem.
I added more comments and updated the error message.
Please, feel free to fix them, if you have any suggestions.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
