Re: Command Triggers, v16

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, Thom Brown <thombrown(at)gmail(dot)com>
Subject: Re: Command Triggers, v16
Date: 2012-03-16 09:40:46
Message-ID: m24ntog9y9.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On Thursday, March 15, 2012 10:58:49 PM Dimitri Fontaine wrote:
>> I tricked that in the grammar, the type is called cmdtrigger but I
>> though it wouldn't be a good choice for the SQL statement.
> Hm. I am decidedly unhappy with that grammar hackery... But then maybe I am
> squeamish.

It's easy to remove that hack and get back to having the user visible
type be cmdtrigger, but as this type only serves as a marker for PL
compiling function (validate handler) I though having a user friendly
name was important here.

>> + oid | typname | oid | proname
>> +------+------------+------+------------
>> + 1790 | refcursor | 46 | textin
>> + 3838 | cmdtrigger | 2300 | trigger_in
>> +(2 rows)
> Hm. Wonder if its a good idea to reuse trigger_in. So far we have duplicated
> functions for that.

Again, if we think the use case warrants duplicating code rather than
playing with the type definition, I will do that.

> This will have the effect of calling triggers outside of alphabetic order. I
> don't think thats a good idea even if one part is ANY and the other a specific
> command.
> I don't think there is any reason anymore to separate the two? The only
> callsite seems to look like:

The idea is to have a predictable ordering of command triggers. The code
changed in the patch v16 (you pasted code from git in between v15 and
v16, I cleaned it up) and is now easier to read:

case CMD_TRIGGER_FIRED_BEFORE:
whenstr = "BEFORE";
procs[0] = cmd->before_any;
procs[1] = cmd->before;
break;

case CMD_TRIGGER_FIRED_AFTER:
whenstr = "AFTER";
procs[0] = cmd->after;
procs[1] = cmd->after_any;
break;

So it's BEFORE ANY then BEFORE command then AFTER command then AFTER
ANY. That's an arbitrary I made and we can easily reconsider. Triggers
are called in alphabetical order in each “slot” here.

In my mind it makes sense to have ANY triggers around the specific
triggers, but it's hard to explain why that feels better.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2012-03-16 09:58:29 Re: Proposal: Create index on foreign table
Previous Message Andres Freund 2012-03-16 09:40:17 Re: Command Triggers, v16