Stephen Frost wrote:
> > > > Yes, I will push these unless somebody objects soon, as they seem
> > > > perfectly reasonable to me. The only troubling thing is that there is
> > > > no regression test for this kind of thing in event triggers (i.e. verify
> > > > which command tags get support and which don't), which seems odd to me.
> > > > Not these patches's fault, though, so I'm not considering adding any ATM.
> > >
> > > Ugh. I dislike that when we say an event trigger will fire before
> > > 'GRANT' what we really mean is "GRANT when it's operating against a
> > > local object". At the minimum we absolutely need to be very clear in
> > > the documentation about that limitation. Perhaps there is something
> > > already which reflects that, but I don't see anything in the patch
> > > being added about that.
> >
> > Hmm, good point, will give this some thought. I'm thinking perhaps we
> > can add a table of which object types are supported for generic commands
> > such as GRANT, COMMENT and SECURITY LABEL.
>
> That sounds like a good idea.
Here's a patch. I noticed that the introduction to event trigger
already says they only act on local objects:
The ddl_command_start event occurs just before the execution of
a CREATE, ALTER, DROP, SECURITY LABEL, COMMENT, GRANT or REVOKE
command. No check whether the affected object exists or doesn't
exist is performed before the event trigger fires. As an
exception, however, this event does not occur for DDL commands
targeting shared objects — databases, roles, and tablespaces —
or for commands targeting event triggers themselves.
So adding more text to the same effect would be repetitive. I added a
sixth column "Notes" to the table of supported command tags vs. events,
with the text "Only for local objects" next to the four commands being
added here.
I think it's fair to push this patch as is.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
