Re: Command Triggers

On Thu, Feb 16, 2012 at 5:38 PM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
>> Another option would be to add a PGC_SUSET boolean GUC that can be
>> used to disable command triggers.  I think that might be more
>> flexible, not to mention useful for recursion prevention.
>
> Wait, we already have ALTER TRIGGER bob ON ANY COMMAND SET DISABLED;
> which I had forgotten about in the previous answer, so I think we're
> good as it is.  That's how I prevented recursion in some of my tests
> (not included in the regress tests, was using INSTEAD OF).

Eh, so what happens then if someone sets a command trigger on ALTER TRIGGER?

>>>  DROP TRIGGER bob ON ALL COMMANDS;
>>
>> Uh, hold on.  Creating a trigger on multiple commands ought to only
>> create one entry in pg_cmdtrigger.  If you drop it, you drop the whole
>> thing.  Changing which commands the trigger applies to would be the
>> job for ALTER, not DROP.  But note that we have no similar
>> functionality for regular triggers, so I can't think we really need it
>> here either.
>
> Why would we do it that way (a single entry for multiple commands)?  The
> way it is now, it's only syntactic sugar, so I think it's easier to
> implement, document and use.

Well, for one thing, it's consistent with how we handle it for regular
triggers. For two things, if you create an object named bob, you
expect to end up with an object named bob - not 47 objects (or
whatever) that are all named bob. Also, suppose you create a trigger
on ALL COMMANDS, and then a new version of PG adds a new command.
When you dump and reload, do you expect to end up with a trigger on
all commands that existed in the old version, or all the commands that
exist in the new version? Or conversely, suppose we get rid of a
command in a future release. How will we handle that? I can't think
of another example of where a CREATE command creates multiple objects
like that.

>>> So do you prefer lots of InitCommandContext() or adding another parameter
>>> to almost all functions called by standard_ProcessUtility()?
>>
>> Blech.  Maybe we should just have CommandFiresTriggers initialize it
>> if not already done?
>
> Thing is, in a vast number of cases (almost of ALTER OBJECT name,
> namespace and owner) you don't have the Node * parse tree any more from
> the place where you check for CommandFiresTriggers(), so you can't
> initialize there. That's part of the fun.

Hmm, I'll look at this in more detail next time through.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company