On Wed, 2021-10-20 at 10:32 -0700, Mark Dilger wrote: > I'd like to have a much clearer understanding of Noah's complaint > first. There are multiple things to consider: (1) the role which > owns the trigger, (2) the role which is performing an action which > would cause the trigger to fire, (3) the set of roles role #1 belongs > to, (4) the set of roles role #1 has ADMIN privilege on, (5) the set > of roles that role #2 belongs to, and (6) the set of roles that role > #2 has ADMIN privilege on. Maybe more? > > And that's before we even get into having roles own other roles, > which the event trigger patches *do not depend on*. In the patch set > associated with this thread, the event trigger stuff is in patches > 0014 and 0015. The changes to CREATEROLE and role ownership are not > until patches 0019, 0020, and 0021. (I'm presently writing another > set of emails to split this all into four threads/patch sets.) > > I'd like to know precisely which combinations of these six things are > objectionable, and why. There may be a way around the objections > without needing to create new user options or new privileged roles.
I can't speak for Noah, but my interpretation is that it would be surprising if GRANT/REVOKE or membership in an ordinary role had effects other than "permission denied" errors. It might make sense for event trigger firing in all the cases we can think of, but it would certainly be strange if we started accumulating a collection of behaviors that implicitly change when you move in or out of a role. That's pretty general, so to answer your question: it seems like a problem to use #3-6 in the calculation about whether to fire an event trigger. However, if we have a concept of role *ownership*, that's something new. It may be less surprising to use that to determine additional behaviors, like whether event triggers fire. We can also consider adding some additional language to the CREATE EVENT TRIGGER syntax to make it more explicit what the scope is. For instance: CREATE EVENT TRIGGER name ON event [ FOR {ALL|OWNED} ROLES ] [ WHEN filter_variable IN (filter_value [, ... ]) [ AND ... ] ] EXECUTE { FUNCTION | PROCEDURE } function_name() For a superuser ALL and OWNED would be the same, but regular users would need to specify "FOR OWNED ROLES" or they'd get an error. Regards, Jeff Davis