Command Triggers, v16

On 15 March 2012 18:13, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Hi,

I guess it's time to start a new thread here. Please find attached
version 16 of the command trigger patch, with augmented documentation
and “magic variable” support (TG_WHEN, TG_OBJECTID and such).

The current version of the patch only supports PLpgSQL, I need to add
support for the other languages in core but I though Thom would like to
be able to play with a new patch before I finish plpython, plperl and
pltcl support.

This patch also includes edits following latest reviews from both Thom,
Andres and Robert, in particular ANY command triggers are now called
from the same place as specific command triggers and receive the same
parameters.

Good to see that ANY COMMAND triggers contain everything the specific
triggers have. I've completed a complete re-run of all my testing.

Note: incremental patch attached for the following section...

-----START----

The docs have an excessive opening <varlistentry> tag. The docs also
list ALTER CAST as an option, which it isn't. There's an old version
of a paragraph included, immediately followed by its revised version.
It begins with "Triggers on ANY command...".

The example given for the abort_any_command function has a typo. The
RAISE statement should have a comma after the closing single quote
instead of %.

In doc/src/sgml/plpgsql.sgml:

“The command trigger function return's value is not used.”
should be
“The command trigger function’s return value is not used.”

“This example trigger just raise a...”
should be
“This example trigger just raises a...”

The example procedure isn't called correctly in the create command
trigger statement below it. It refers to it at "any_snitch", but the
function is just named "snitch". Also the style is inconsistent with
the other trigger functions further up the page, such as putting the
function language last, showing the return type on the same line as
the CREATE FUNCTION line and using upper-case lettering for keywords.

I don’t understand how functions can return a type of “command
trigger”. This certainly works, but I’ve never seen a type consisting
of more than one word. Could you explain this for me? This is also
at odds with the error message in src/backend/commands/cmdtrigger.c:

errmsg("function \"%s\" must return type \"trigger\"",

Should be “command trigger” as a regular trigger can’t be used on
command triggers.

----END----

At this moment in time, CTAS is still outstanding. Is the plan to try
to get that in for this release, or as an enhancement in 9.3?

I don’t know if this was a problem before that I didn’t spot
(probably), but triggers for both ANY COMMAND and ALTER FOREIGN TABLE
show a command tag of ALTER TABLE for ALTER FOREIGN TABLE statements
where the column is renamed:

thom@test=# ALTER FOREIGN TABLE test.dict2 RENAME COLUMN word TO words;
NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER
TABLE' objectid=16569 schemaname='test' objectname='dict2'
NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='ALTER TABLE'
objectid=16569 schemaname='test' objectname='dict2'
NOTICE: Command trigger: tg_when='AFTER' cmd_tag='ALTER TABLE'
objectid=16569 schemaname='test' objectname='dict2'
NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='ALTER TABLE'
objectid=16569 schemaname='test' objectname='dict2'
ALTER TABLE

I don’t think this is the fault of the trigger code because it
actually says ALTER TABLE at the bottom, suggesting it’s something
already present. This isn’t the case when adding or dropping columns.
Any comments?

Altering the properties of a function (such as cost, security definer,
whether it’s stable etc) doesn’t report the function’s OID:

thom@test=# ALTER FUNCTION test.testfunc2() COST 77;
NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER
FUNCTION' objectid=<NULL> schemaname='test' objectname='testfunc2'
NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='ALTER FUNCTION'
objectid=<NULL> schemaname='test' objectname='testfunc2'
NOTICE: Command trigger: tg_when='AFTER' cmd_tag='ALTER FUNCTION'
objectid=<NULL> schemaname='test' objectname='testfunc2'
NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='ALTER
FUNCTION' objectid=<NULL> schemaname='test' objectname='testfunc2'
ALTER FUNCTION

I get a garbage objectname for AFTER ANY COMMAND triggers on ALTER
TEXT SEARCH DICTIONARY when changing its options. It doesn’t show it
in the below example because I can’t get it displaying in plain text,
but where the objectname is blank is where I’m seeing unicode a square
containing “0074” 63 times in a row:

thom@test=# ALTER TEXT SEARCH DICTIONARY testnew.test_stem2 (
StopWords = dutch );
NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER TEXT
SEARCH DICTIONARY' objectid=16617 schemaname='testnew'
objectname='test_stem2'
NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='ALTER TEXT SEARCH
DICTIONARY' objectid=16617 schemaname='testnew'
objectname='test_stem2'
NOTICE: Command trigger: tg_when='AFTER' cmd_tag='ALTER TEXT SEARCH
DICTIONARY' objectid=16617 schemaname='testnew'
objectname='test_stem2'
NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='ALTER TEXT
SEARCH DICTIONARY' objectid=16617 schemaname='testnew'
objectname=' '
ALTER TEXT SEARCH DICTIONARY

Specific command triggers on ALTER VIEW don’t work at all:

thom@test=# ALTER VIEW view_test OWNER TO test;
NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER VIEW'
objectid=16625 schemaname='public' objectname='view_test'
NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='ALTER VIEW'
objectid=16625 schemaname='public' objectname='view_test'
ALTER VIEW

Command triggers that fire for CREATE RULE show a schema, but DROP
RULE doesn’t. Which is it?:

thom@test=# CREATE RULE notify_test AS ON UPDATE TO seq_table DO ALSO
NOTIFY test; -- support for testing DROP RULE
NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='CREATE
RULE' objectid=<NULL> schemaname='public' objectname='notify_test'
NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='CREATE RULE'
objectid=<NULL> schemaname='public' objectname='notify_test'
NOTICE: Command trigger: tg_when='AFTER' cmd_tag='CREATE RULE'
objectid=16706 schemaname='public' objectname='notify_test'
NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='CREATE RULE'
objectid=16706 schemaname='public' objectname='notify_test'
CREATE RULE
thom@test=# DROP RULE notify_test ON seq_table;
NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='DROP RULE'
objectid=16706 schemaname='<NULL>' objectname='notify_test'
NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='DROP RULE'
objectid=16706 schemaname='<NULL>' objectname='notify_test'
NOTICE: Command trigger: tg_when='AFTER' cmd_tag='DROP RULE'
objectid=<NULL> schemaname='<NULL>' objectname='notify_test'
NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='DROP RULE'
objectid=<NULL> schemaname='<NULL>' objectname='notify_test'
DROP RULE

This same behaviour exists for DROP TRIGGER.

Regards

Thom

Thanks for testing this new version (again).

A quick answer now, I'll send another patch tomorrow.

Thom Brown <thombrown@gmail.com> writes:

I don’t understand how functions can return a type of “command
trigger”. This certainly works, but I’ve never seen a type consisting
of more than one word. Could you explain this for me? This is also

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.

at odds with the error message in src/backend/commands/cmdtrigger.c:

errmsg("function \"%s\" must return type \"trigger\"",

I realized I needed another trigger like data type after I had worked
this message, I need to get back on it, thanks.

At this moment in time, CTAS is still outstanding. Is the plan to try
to get that in for this release, or as an enhancement in 9.3?

The plan is to get CTAS as a utility command in 9.2 then update the
command trigger patch to benefit from the new situation. We've been
wondering about making its own commit fest entry for that patch, it's
now clear in my mind, that needs to happen.

Stay tuned, follow up email due tomorrow.
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:

At this moment in time, CTAS is still outstanding. Is the plan to try
to get that in for this release, or as an enhancement in 9.3?

The plan is to get CTAS as a utility command in 9.2 then update the
command trigger patch to benefit from the new situation. We've been
wondering about making its own commit fest entry for that patch, it's
now clear in my mind, that needs to happen.

https://commitfest.postgresql.org/action/patch_view?id=823

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

On 15 March 2012 22:06, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:

At this moment in time, CTAS is still outstanding.  Is the plan to try
to get that in for this release, or as an enhancement in 9.3?

The plan is to get CTAS as a utility command in 9.2 then update the
command trigger patch to benefit from the new situation. We've been
wondering about making its own commit fest entry for that patch, it's
now clear in my mind, that needs to happen.

 https://commitfest.postgresql.org/action/patch_view?id=823

Looks like the ctas-on-command-triggers-01.patch patch needs re-basing.

Thom

On Thursday, March 15, 2012 11:41:21 PM Thom Brown wrote:

On 15 March 2012 22:06, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:

At this moment in time, CTAS is still outstanding. Is the plan to try
to get that in for this release, or as an enhancement in 9.3?

The plan is to get CTAS as a utility command in 9.2 then update the
command trigger patch to benefit from the new situation. We've been
wondering about making its own commit fest entry for that patch, it's
now clear in my mind, that needs to happen.

https://commitfest.postgresql.org/action/patch_view?id=823

Looks like the ctas-on-command-triggers-01.patch patch needs re-basing.

I can do that - but imo the other patch (not based on the command triggers
stuff) is the relevant for now as this patch ought to be applied before
command triggers. It doesn't seem to make too much sense to rebase it
frequently as long as the command triggers patch isn't stable...

Any reason you would prefer it being rebased?

Andres

* Thom Brown wrote:

I don�t understand how functions can return a type of �command
trigger�. This certainly works, but I�ve never seen a type consisting
of more than one word. Could you explain this for me? This is also

postgres=> with types (name) as
postgres-> (select format_type(oid, NULL) from pg_type)
postgres-> select name from types where name like '% %'
postgres-> and not name like '%[]';
name
-----------------------------
double precision
character varying
time without time zone
timestamp without time zone
timestamp with time zone
time with time zone
bit varying
(7 Zeilen)

I think these are all specially handled in the parser.

--
Christian Ullrich

On 16 March 2012 08:13, Andres Freund <andres@anarazel.de> wrote:

On Thursday, March 15, 2012 11:41:21 PM Thom Brown wrote:

Looks like the ctas-on-command-triggers-01.patch patch needs re-basing.

I can do that - but imo the other patch (not based on the command triggers
stuff) is the relevant for now as this patch ought to be applied before
command triggers. It doesn't seem to make too much sense to rebase it
frequently as long as the command triggers patch isn't stable...

Any reason you would prefer it being rebased?

Using latest Git master without any additional patches, I can't get it to apply:

Hunk #1 FAILED at 16.
Hunk #2 succeeded at 22 (offset -1 lines).
1 out of 2 hunks FAILED -- saving rejects to file
src/include/commands/tablecmds.h.rej

Thom

Hi,

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.

+ 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.

@@ -482,12 +494,21 @@ ListCommandTriggers(CommandContext cmd)

switch (form->ctgtype)
{

case CMD_TRIGGER_FIRED_BEFORE:
-               cmd->before = lappend_oid(cmd->before, form->ctgfoid);
+           {
+               if (list_any_triggers)
+                   cmd->before_any = lappend_oid(cmd->before_any,
form->ctgfoid); +               else
+                   cmd->before = lappend_oid(cmd->before, form->ctgfoid);

break;

-
...
+       case CMD_TRIGGER_FIRED_BEFORE:
+       {
+           whenstr = "BEFORE";
+
+           foreach(cell, cmd->before_any)
+           {
+               Oid proc = lfirst_oid(cell);
+
+               call_cmdtrigger_procedure(cmd, (RegProcedure)proc,
whenstr); +           }
+           foreach(cell, cmd->before)
+           {
+               Oid proc = lfirst_oid(cell);
+
+               call_cmdtrigger_procedure(cmd, (RegProcedure)proc,
whenstr); +           }
+           break;
+       }

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:

632- default:
633: ListCommandTriggers(cmd, true); /* list ANY command triggers */
634: ListCommandTriggers(cmd, false); /* and triggers for this
command tag */

Andres

On Friday, March 16, 2012 09:30:58 AM Thom Brown wrote:

On 16 March 2012 08:13, Andres Freund <andres@anarazel.de> wrote:

On Thursday, March 15, 2012 11:41:21 PM Thom Brown wrote:

Looks like the ctas-on-command-triggers-01.patch patch needs re-basing.

I can do that - but imo the other patch (not based on the command
triggers stuff) is the relevant for now as this patch ought to be
applied before command triggers. It doesn't seem to make too much sense
to rebase it frequently as long as the command triggers patch isn't
stable...

Any reason you would prefer it being rebased?

Using latest Git master without any additional patches, I can't get it to
apply:

Hunk #1 FAILED at 16.
Hunk #2 succeeded at 22 (offset -1 lines).
1 out of 2 hunks FAILED -- saving rejects to file
src/include/commands/tablecmds.h.rej

Did you read the paragraph above?

Andres

On 15 March 2012 21:58, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Thom Brown <thombrown@gmail.com> writes:

I don’t understand how functions can return a type of “command
trigger”.  This certainly works, but I’ve never seen a type consisting
of more than one word.  Could you explain this for me?  This is also

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.

Christian sent me a message mentioning that we've pretty much always
had data types consisting of more than one word (e.g. timestamp
without time zone). So I completely retract my question as it's
obviously nonsense.

Thom

On 16 March 2012 08:45, Andres Freund <andres@anarazel.de> wrote:

On Friday, March 16, 2012 09:30:58 AM Thom Brown wrote:

On 16 March 2012 08:13, Andres Freund <andres@anarazel.de> wrote:

On Thursday, March 15, 2012 11:41:21 PM Thom Brown wrote:

Looks like the ctas-on-command-triggers-01.patch patch needs re-basing.

I can do that - but imo the other patch (not based on the command
triggers stuff) is the relevant for now as this patch ought to be
applied before command triggers. It doesn't seem to make too much sense
to rebase it frequently as long as the command triggers patch isn't
stable...

Any reason you would prefer it being rebased?

Using latest Git master without any additional patches, I can't get it to
apply:

Hunk #1 FAILED at 16.
Hunk #2 succeeded at 22 (offset -1 lines).
1 out of 2 hunks FAILED -- saving rejects to file
src/include/commands/tablecmds.h.rej

Did you read the paragraph above?

Yes, but I don't think I'm clear on what you mean. Are you saying I
should use ctas-01.patch instead of ctas-on-command-triggers-01.patch?
If so, that patch results in me not being able to apply Dimitri's
command triggers patch. And I thought that patch doesn't actually
cause triggers to fire on CTAS?

Thom

On Friday, March 16, 2012 09:55:10 AM Thom Brown wrote:

On 16 March 2012 08:45, Andres Freund <andres@anarazel.de> wrote:

On Friday, March 16, 2012 09:30:58 AM Thom Brown wrote:

On 16 March 2012 08:13, Andres Freund <andres@anarazel.de> wrote:

On Thursday, March 15, 2012 11:41:21 PM Thom Brown wrote:

Looks like the ctas-on-command-triggers-01.patch patch needs
re-basing.

I can do that - but imo the other patch (not based on the command
triggers stuff) is the relevant for now as this patch ought to be
applied before command triggers. It doesn't seem to make too much
sense to rebase it frequently as long as the command triggers patch
isn't stable...

Any reason you would prefer it being rebased?

Using latest Git master without any additional patches, I can't get it
to apply:

Hunk #1 FAILED at 16.
Hunk #2 succeeded at 22 (offset -1 lines).
1 out of 2 hunks FAILED -- saving rejects to file
src/include/commands/tablecmds.h.rej

Did you read the paragraph above?

Yes, but I don't think I'm clear on what you mean. Are you saying I
should use ctas-01.patch instead of ctas-on-command-triggers-01.patch?
If so, that patch results in me not being able to apply Dimitri's
command triggers patch. And I thought that patch doesn't actually
cause triggers to fire on CTAS?

Well. Why do you want to apply them concurrently? As far as I understand the
plan is to get ctas-as-utility merged with master and then let dim rebase the
ddl trigger patch.
The ctas-as-utility stuff imo is worthy of being applied independently of DDL
triggers. The current duplication in the code lead to multiple bugs already.

Andres

Andres Freund <andres@anarazel.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

Thom Brown <thombrown@gmail.com> writes:

Note: incremental patch attached for the following section...

Applied, thanks!

I don’t know if this was a problem before that I didn’t spot
(probably), but triggers for both ANY COMMAND and ALTER FOREIGN TABLE
show a command tag of ALTER TABLE for ALTER FOREIGN TABLE statements
where the column is renamed:

I don’t think this is the fault of the trigger code because it
actually says ALTER TABLE at the bottom, suggesting it’s something
already present. This isn’t the case when adding or dropping columns.
Any comments?

We're building command trigger on top of the existing code. From the
grammar we can see that an alter table and an alter foreign table are
processed as the same command.

AlterForeignTableStmt:
ALTER FOREIGN TABLE relation_expr alter_table_cmds
{
AlterTableStmt *n = makeNode(AlterTableStmt);

So while I think we could want to revisit that choice down the road, I
don't think that's for the command triggers patch to do it.

Altering the properties of a function (such as cost, security definer,
whether it’s stable etc) doesn’t report the function’s OID:

That's now fixed. I've checked that all the other places where we're
saying objectId = InvalidOid are related to before create or after drop
commands.

I get a garbage objectname for AFTER ANY COMMAND triggers on ALTER
TEXT SEARCH DICTIONARY when changing its options. It doesn’t show it
in the below example because I can’t get it displaying in plain text,
but where the objectname is blank is where I’m seeing unicode a square
containing “0074” 63 times in a row:

I couldn't reproduce, but I've spotted the problem in the source, and
fixed it. I could find a couple other places that should have been using
pstrdup(NameStr(…)) too, and fixed them. I added a test.

Specific command triggers on ALTER VIEW don’t work at all:

Can't reproduce, and that's already part of the regression tests.

Command triggers that fire for CREATE RULE show a schema, but DROP
RULE doesn’t. Which is it?:

Oh, both RULE and TRIGGER are not qualified, and I was filling the
schemaname with the schema of the relation they refer to in the CREATE
command, and had DROP correctly handling the TRIGGER case.

That's now fixed, schemaname is NULL in all cases here.

You can read the changes here:

https://github.com/dimitri/postgres/compare/5e8e37922d...144d870162

And I've been attaching an incremental patch too. Next patch revision is
expected later today with support for plpython, plperl and pltcl.

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

On 16 March 2012 11:42, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Thom Brown <thombrown@gmail.com> writes:

Specific command triggers on ALTER VIEW don’t work at all:

Can't reproduce, and that's already part of the regression tests.

This was a problem my side (a mistake I made previously) as I hadn't
added this particular one into my list of created command triggers. I
had then seen the triggers fire, but forgot to go back and remove my
statement from the draft email. Apologies.

And I've been attaching an incremental patch too. Next patch revision is
expected later today with support for plpython, plperl and pltcl.

Okay, I shalln't do any more testing until the next patch. I should
probably have worked on automating my tests more, but never got round
to it.

Thom

Thom Brown <thombrown@gmail.com> writes:

Okay, I shalln't do any more testing until the next patch. I should
probably have worked on automating my tests more, but never got round
to it.

make installcheck :)

That said, your test allow to spot OID problems that we can't add in the
regression tests (OID being too volatile would break them), and I have
to look at how to add regression tests for the other pls support…

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

On 16 March 2012 12:07, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Thom Brown <thombrown@gmail.com> writes:

Okay, I shalln't do any more testing until the next patch.  I should
probably have worked on automating my tests more, but never got round
to it.

 make installcheck :)

That said, your test allow to spot OID problems that we can't add in the
regression tests (OID being too volatile would break them), and I have
to look at how to add regression tests for the other pls support…

Yes, that's why I don't use the regression tests. :)

Thom

Andres Freund <andres@anarazel.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.

Multi-word type names are a serious pain in the ass; they require
hackery in a lot of places. We support the ones that the SQL spec
requires us to, but I will object in the strongest terms to inventing
any that are not required by spec. I object in even stronger terms to
the incredibly klugy way you did it here.

If you think "cmdtrigger" isn't a good name maybe you should have
picked a different one to start with.

While I'm looking at the grammar ... it also seems like a serious
PITA from a maintenance standpoint that we're now going to have to
adjust the CREATE COMMAND TRIGGER productions every time somebody
thinks of a new SQL command. Maybe we should drop this whole idea
of specifying which commands a trigger acts on at the SQL level,
and just have one-size-fits-all command triggers. Or perhaps have
the selection be on the basis of strings that are matched to command
tags, instead of grammar constructs.

regards, tom lane

On Friday, March 16, 2012 02:50:55 PM Tom Lane wrote:

While I'm looking at the grammar ... it also seems like a serious
PITA from a maintenance standpoint that we're now going to have to
adjust the CREATE COMMAND TRIGGER productions every time somebody
thinks of a new SQL command.

I don't find that argument really convincing. The current state of the patch
will often require attention to command triggers anyway...

Andres