Re: Command Triggers, patch v11

Hi,

I believed I fixed all known glitches (all were minor quibbles here and
there because of me trying to work too fast), all I know about remaining
in the TODO list is passing the arguments the classic trigger way, I'm
waiting on a "go" from Tom on the way to do it, but well, will try on my
own if that's how busy he is (I just had enough on my plate to be able
to wait for an ack from him).

Thom Brown <thom(at)linux(dot)com> writes:
>> already covered?  Did you try make installcheck?
> Yes, but I felt it better that I come up with my own separate tests.

Ok, great then :)

> Specific command triggers agains ALTER FUNCTION (i.e. not ANY COMMAND)
> don’t fire for any changes except renaming, changing owner or changing
> schema. Everything else fails to trigger (cost, rows, setting
> configuration parameters, setting strict, security invoker etc.).:

> Specific command triggers on ALTER SEQUENCE don’t fire:
> Specific command triggers on ALTER TABLE don’t fire for renaming columns:
> Also renaming attributes doesn’t fire specific triggers:
> Specific command triggers on ALTER VIEW don’t fire for any type of change:

All previous cases and those are now fixed.

> Is there a reason why we can't provide the OID for ANY COMMAND... if
> it's available? I'm guessing it would end up involving having to
> special case for every command. :/

It's not available where it's implemented, to get the OID you need a
full integration into each code path and the idea being the ANY command
trigger is still to be able to catch more DDL than we are supporting for
specific commands.

>> Roles are global objects, you don't want the behavior of role commands
>> to depend on which database you happen to have been logged in when
>> issuing the command.  That would call for removing the ANY command
>> support for them too, but I can't seem to decide about that.
>
> If that's your reasoning, then it would make sense to remove ANY
> command support for it too.

I did that in the attached, version 14 of the patch.

>>> There doesn’t appear to be command trigger support for ALTER LARGE OBJECT.
>
> *shrug* But ANY COMMAND triggers fire for it. So I'd say either
> remove support for that, or add a specific trigger.

I tried adding that, but it's implemented in catalog/ and my tries at
having the catalog include file include the commands/cmdtrigger.h file
have been failing. I don't know how to implement that, dismissed for
now.

>> [CASCADE will not run the command triggers for cascaded objects]
> If these are all expected, does it in any way compromise the
> effectiveness of DDL triggers in major use-cases?

I don't think so. When replicating the replica will certainly drop the
same set of dependent objects, for example. Auditing is another story.
Do we want to try having cascaded object support in drop commands?

As the current code is not going through standard_ProcessUtility() in
such cases, I don't think it's possible to do for 9.2.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support