On Wed, Dec 04, 2019 at 06:15:52PM +0900, Arthur Zakirov wrote:
> On 2019/12/04 17:15, Michael Paquier wrote:
>> FWIW, I am not much a fan of that part because the output generated by
>> the description is most likely not compatible with the grammar
>> supported.
>
> 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.
> Sure! But I'm not sure that I understood the idea. Do you mean the patch
> which adds a TAP test? It can initialize two clusters, in first it renames
> some system objects and changes their ACLs. And finally the test runs
> pg_upgrade which will fail.
A TAP test won't help here because the idea is to create a patch for
HEAD which willingly introduces changes for system objects, where the
source binaries have ACLs on object types which are broken on the
target binaries with the custom patch. That's to make sure that all
the logic which would get added to pu_upgrade is working correctly.
Other ideas are welcome.
--
Michael