Re: Command Triggers, patch v11

On 26 February 2012 19:49, Thom Brown <thom(at)linux(dot)com> wrote:
> On 26 February 2012 14:12, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>> Thanks for your further testing!
>>
>> Thom Brown <thom(at)linux(dot)com> writes:
>>> Further testing reveals a problem with FTS configurations when using
>>> the example function provided in the docs:
>>
>> Could you send me your tests so that I add them to the proper regression
>> test?  I've been lazy on one or two object types and obviously that's
>> where I have to check some more.
>
> Which tests?  The FTS Config test was what I posted before.  I haven't
> gone to any great effort to set up tests for each command.  I've just
> been making them up as I go along.
>
>>> What is DROP ASSERTION?  It's showing as a valid command for a command
>>> trigger, but it's not documented.
>>
>> It's a Not Implemented Feature for which we have the grammar support to
>> be able to fill a standard compliant checkbox, or something like that.
>> It could be better for me to remove explicit support for it in the
>> command triggers patch?
>
> Well considering there are commands that exist which we don't allow
> triggers on, it seems weird to support triggers on commands which
> aren't implemented.  DROP ASSERTION doesn't appear anywhere else in
> the documentation, so I can't think of how supporting a trigger for it
> could be useful.
>
>>> I've noticed that ALTER <object> name OWNER TO role doesn't result in
>>> any trigger being fired except for tables.
>>>
>>> ALTER OPERATOR FAMILY .... RENAME TO ... doesn't fire command triggers.
>>>
>>> ALTER OPERATOR CLASS with RENAME TO or OWNER TO doesn't fire command
>>> triggers, but with SET SCHEMA it does.
>>
>> It seems I've forgotten to add some support here, that happens in
>> alter.c and is easy enough to check and complete, thanks for the
>> testing.
>
> So would the fix cover many cases at once?
>
>>> I'll hold off on testing any further until a new patch is available.
>>
>> That should happen soon. Ah, the joys of coding while kids are at home
>> thanks to school holidays. I can't count how many times I've been killed
>> by a captain and married to a princess while writing that patch, sorry
>> about those hiccups here.
>
> Being killed by a captain does make things more difficult, yes.

I've got a question regarding the function signatures required for
command triggers, and apologies if it's already been discussed to
death (I didn't see all the original conversations around this).
These differ from regular trigger functions which don't require any
arguments, and instead use special variables. Why aren't we doing the
same for command triggers? So instead of having the parameters
tg_when, cmd_tag, objectid, schemaname and objectname, using pl/pgsql
as an example, we'd have the variables TG_WHEN (already exists), TG_OP
(already exists and equivalent to cmd_tag), TG_RELID (already exists,
although maybe not directly equivalent), TG_REL_SCHEMA (doesn't exist
but would replace schemaname) and TG_RELNAME (this is actually
deprecated but could be re-used for this purpose).

Advantages of implementing it like this is that there's consistency in
the trigger system, it's easier as no function parameters required,
and any future options you may wish to add won't break functions from
previous versions, meaning more room for adding stuff later on.

Disadvantages are that there's more maintenance overhead for
supporting multiple languages using special variables.

--
Thom