On 17.12.2019 11:10, Arthur Zakirov wrote:
On 2019/12/05 11:31, Michael Paquier wrote:
On Wed, Dec 04, 2019 at 06:15:52PM +0900, Arthur Zakirov wrote:
Ah, I thought that pg_identify_object() gives properly quoted identity, and
it could be used to make SQL script.
It depends on the object type. For columns I can see in your patch
that you are using a dedicated code path, but I don't think that we
should assume that there is an exact one-one mapping between the
object type and the grammar of GRANT/REVOKE for the object type
supported because both are completely independent things and
facilities. Take for example foreign data wrappers. Of course for
this patch this depends if we have system object of the type which
would be impacted. That's not the case of foreign data wrappers and
likely it will never be, but there is no way to be sure that this
won't get broken silently in the future.
I attached new version of the patch. It still uses pg_identify_object(), I'm not sure about other ways to build identities yet.
Michael, do I understand it correctly that your concerns about pg_identify_object() relate only to the revoke sql script?
Now, I tend to agree that we should split this patch into two separate parts, to make it simpler.
The first patch is to find affected objects and print warnings and the second is to generate script.
I added "test_rename_catalog_objects.patch" and "test_add_acl_to_catalog_objects.sql". So testing steps are following:
- initialize new source instance (e.g. pg v12) and run "test_add_acl_to_catalog_objects.sql" on it
- apply "pg_upgrade_ACL_check_v6.patch" and "test_add_acl_to_catalog_objects.sql" for HEAD
- initialize new target instance for HEAD
- run pg_upgrade, it should create revoke_objects.sql file
"test_rename_catalog_objects.patch" should be applied after "pg_upgrade_ACL_check_v6.patch".
Renamed objects are the following:
- table pg_subscription -> pg_sub
- columns pg_subscription.subenabled -> subactive, pg_subscription.subconninfo -> subconn
- view pg_stat_subscription -> pg_stat_sub
- columns pg_stat_subscription.received_lsn -> received_location, pg_stat_subscription.latest_end_lsn -> latest_end_location
- function pg_stat_get_subscription -> pg_stat_get_sub
- language sql -> pgsql
I've tried to test it. Script is attached.
Described test case works. If a new cluster contains renamed objects,
pg_upgrade --check successfully finds them and generates a script to revoke non-default ACL. Though, without test_rename_catalog_objects.patch,
pg_upgrade --check still generates the same message, which is false positive.
I am going to fix it and send the updated patch later this week.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
