Re: Allow database owners to CREATE EVENT TRIGGER

> You have a bad drop table cleanup command, and I’d drop the entire alter
event trigger owner test.

My bad, removed the bad drop table and also removed the alter owner test.

> The other thing I’m wondering, but haven’t gotten around to testing, is
whether a role affected by the event trigger is able to just drop the
trigger given this implementation. I always get member/member-of dynamics
confused. Having member (possibly via set role…) trying to drop the
trigger would be good to prove that it isn’t allowed.

So this one is a problem. If we do `grant evtrig_owner to member` then
`member` will be able to drop the event trigger, which is no good. There
are 2 option to solve this:

1. Do `grant evtrig_owner to member with inherit false`, then `member` will
not be able to drop the event trigger. I've updated the tests to reflect
that.
2. `create role member noinherit`, which won't let `member` drop the event
trigger with a regular `grant evtrig_owner to member`. But this change is
more invasive towards existing roles.

That being said, it's not a good default behavior to let evtrigger targets
to drop the evtrigger. Should we enforce that only granting with inherit
false will make the role members targets of the evtrigger? Are there any
other options?

On Tue, 22 Apr 2025 at 20:18, David G. Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>
wrote:

> On Tuesday, April 22, 2025, David G. Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>
> wrote:
>
>> On Tuesday, April 22, 2025, David G. Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>
>> wrote:
>>
>>> On Tuesday, April 22, 2025, Steve Chavez <steve(at)supabase(dot)io> wrote:
>>>
>>>> > alter event trigger command which doesn’t need to be exercised here
>>>>
>>>> That part does need to be tested, I modified
>>>> `AlterEventTriggerOwner_internal` to allow altering owners to regular
>>>> users. Before it was only restricted to superusers.
>>>>
>>>
>> Ok. I missed this.
>>
>
> Sorry for the self-reply but this nagged at me.
>
> It’s probably not a big deal either way, but the prior test existed to
> ensure that a superuser couldn’t do something they are otherwise are always
> permitted to do - assign object to whomever they wish. So
> event_trigger.sql had a test that errored showing this anomaly. You moved
> the test and now are proving it doesn’t error. But it is not expected to
> error; and immediately above you already show that a non-superuser can be
> an owner. We don’t need a test to show a superuser demonstrating their
> normal abilities.
>
> IOW, select test cases around the feature as it is implemented now, not
> its history. A personal one-off test to ensure that no super-user
> prohibitions remained will suffice to make sure all such code that needed
> to be removed is gone.
>
> David J.
>
>