Greetings Michael,
* Michael Paquier (michael@paquier.xyz) wrote:
> On Mon, Nov 18, 2019 at 09:16:06AM +0000, Floris Van Nee wrote:
> > So, if we do it during DROP OWNED BY, we should also handle it
> > during one of the below REVOKE commands. Perhaps DROP OWNED BY
> > already calls one of these functions internally - in that case you
> > can ignore my comment. Just wanted to make sure we catch all
> > possible cases this can occur.
>
> One problem here is that you would need to scan and track down all the
> entries in pg_init_privs if you'd like to make sure that all the
> traces of the role have been removed, so that's really not good for
> performance. The fix I was actually imagining upthread would be to
> add a new type of entry in pg_shdepend linking the role with
> pg_init_privs for the object where an ACL is added. This would
> prevent DROP ROLE to complete with your scenarios, and if the role
> ownerships are dropped we could use the link between the object and
> the role to remove directly the ACL for the role in the object.
> That's not really backpatchable unfortunately. On top of the new
> pg_depend type we would need a new cleanup routine similar to what
> happens for policy ACLs when dropping a role.
Please review the message I just posted on this thread in response to
the original complaint. I really don't think we want these entries to
end up in pg_init_privs in the first place and all of this around the
dependency handling is, at best, an independent thing and not actually
the root cause of this problem that we should be addressing.
I'm not sure that there's a case where this dependency handling would
actually be needed that's being described here, assuming we address the
root issue that these entries are being added in the first place- maybe
there is if an extension's .sql script has a built-in assumption that
some role already exists and then a user goes and drops that role, but
it seems like the right answer there might be to require the user to
drop the extension itself and not the role and we should track the
dependency that way.
Thanks,
Stephen