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
signature.asc
Description: PGP signature