On 15 March 2012 18:13, Dimitri Fontaine <dimi...@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
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f5f2079..13c8c81 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3933,20 +3933,19 @@ SELECT * FROM sales_summary_bytime;
    <title>Triggers on commands</title>
 
    <para>
-    <application>PL/pgSQL</application> can be used to define trigger
-    procedures. A trigger procedure is created with the
+    <application>PL/pgSQL</application> can be used to define command 
+    trigger procedures. A command trigger procedure is created with the
     <command>CREATE FUNCTION</> command, declaring it as a function with
     no arguments and a return type of <type>command trigger</type>.
    </para>
 
   <para>
    When a <application>PL/pgSQL</application> function is called as a
-   trigger, several special variables are created automatically in the
-   top-level block. They are:
+   command trigger, several special variables are created automatically
+   in the top-level block. They are:
 
    <variablelist>
     <varlistentry>
-    <varlistentry>
      <term><varname>TG_TAG</varname></term>
      <listitem>
       <para>
@@ -4002,7 +4001,7 @@ SELECT * FROM sales_summary_bytime;
   </para>
 
   <para>
-   The command trigger function return's value is not used.
+   The command trigger function's return value is not used.
   </para>
 
    <para>
@@ -4014,23 +4013,20 @@ SELECT * FROM sales_summary_bytime;
     <title>A <application>PL/pgSQL</application> Command Trigger Procedure</title>
 
     <para>
-     This example trigger just raise a <literal>NOTICE</literal> message
+     This example trigger simply raises a <literal>NOTICE</literal> message
      each time a supported command is executed.
     </para>
 
 <programlisting>
-create or replace function snitch()
- returns command trigger
- language plpgsql
-as $$
-begin
-  raise notice 'snitch: % % %.% [%]',
+CREATE OR REPLACE FUNCTION snitch() RETURNS command trigger AS $$
+BEGIN
+    RAISE NOTICE 'snitch: % % %.% [%]',
                tg_when, tg_tag, tg_schemaname, tg_objectname, tg_objectid;
-end;
-$$;
+END;
+$$ LANGUAGE plpgsql;
 
-create command trigger snitch_before before any command execute procedure any_snitch();
-create command trigger snitch_after after any command execute procedure any_snitch();
+CREATE COMMAND TRIGGER snitch_before BEFORE ANY COMMAND EXECUTE PROCEDURE snitch();
+CREATE COMMAND TRIGGER snitch_after AFTER ANY COMMAND EXECUTE PROCEDURE snitch();
 </programlisting>
    </example>
   </sect2>
diff --git a/doc/src/sgml/ref/create_command_trigger.sgml b/doc/src/sgml/ref/create_command_trigger.sgml
index fc12d2e..01c7826 100644
--- a/doc/src/sgml/ref/create_command_trigger.sgml
+++ b/doc/src/sgml/ref/create_command_trigger.sgml
@@ -30,7 +30,6 @@ CREATE COMMAND TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFOR
 <phrase>where <replaceable class="parameter">command</replaceable> can be one of:</phrase>
 
     ALTER AGGREGATE
-    ALTER CAST
     ALTER COLLATION
     ALTER CONVERSION
     ALTER DOMAIN
@@ -56,6 +55,7 @@ CREATE COMMAND TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFOR
     ALTER VIEW
     CLUSTER
     CREATE AGGREGATE
+    CREATE CAST
     CREATE COLLATION
     CREATE CONVERSION
     CREATE DOMAIN
@@ -186,14 +186,12 @@ CREATE COMMAND TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFOR
       That leaves out the following list of non supported commands.
      </para>
      <para>
-      Commands that refers to global objects, such as databases, tablespaces
-      and roles are not supported. As command triggers are per-database, it
-      would be weird to affect e.g. a tablespace depending on which database
-      you are connected to.
+      Commands that refer to global objects, such as databases, tablespaces
+      and roles, are not supported.
      </para>
      <para>
       Commands that exercise their own transaction control are only
-      supported in <literal>BEFORE</literal> command triggers, that's the
+      supported in <literal>BEFORE</literal> command triggers.  This is the
       case for <literal>VACUUM</literal>, <literal>CLUSTER</literal>,
       <literal>CREATE INDEX CONCURRENTLY</literal>, and <literal>REINDEX
       DATABASE</literal>.
@@ -220,11 +218,6 @@ CREATE COMMAND TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFOR
       not to be able to take over control from a superuser.
      </para>
      <para>
-      Triggers on ANY command support more commands than just this list, and
-      will provide NULL values for every argument except for the argument
-      that determines whether the trigger was before or after the command
-      event, and the command tag.
-
       Triggers on <literal>ANY</literal> command support more commands than
       just this list, and will only provide the <literal>command
       tag</literal> argument as <literal>NOT NULL</literal>. Supporting more
@@ -238,12 +231,12 @@ CREATE COMMAND TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFOR
     <term><replaceable class="parameter">function_name</replaceable></term>
     <listitem>
      <para>
-      A user-supplied function that is declared as taking 5 arguments of
-      type text, text, oid, text, text and returning void.
+      A user-supplied function that is declared as taking no argument and
+      returning type <literal>command trigger</literal>.
      </para>
      <para>
       If your command trigger is implemented in <literal>C</literal> then it
-      will be called with yet another argument, of
+      will be called with an argument, of
       type <literal>internal</literal>, which is a pointer to
       the <literal>Node *</literal> parse tree.
      </para>
@@ -275,7 +268,7 @@ CREATE COMMAND TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFOR
 CREATE OR REPLACE FUNCTION abort_any_command()
  RETURNS command trigger LANGUAGE plpgsql AS $$
 BEGIN
-  RAISE EXCEPTION 'command % is disabled' % tg_tag;
+  RAISE EXCEPTION 'command % is disabled', tg_tag;
 END;
 $$;
 
diff --git a/src/backend/commands/cmdtrigger.c b/src/backend/commands/cmdtrigger.c
index 6f134d5..f06f6b9 100644
--- a/src/backend/commands/cmdtrigger.c
+++ b/src/backend/commands/cmdtrigger.c
@@ -191,7 +191,7 @@ CreateCmdTrigger(CreateCmdTrigStmt *stmt, const char *queryString)
 	if (funcrettype != CMDTRIGGEROID)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("function \"%s\" must return type \"trigger\"",
+				 errmsg("function \"%s\" must return type \"command trigger\"",
 						NameListToString(stmt->funcname))));
 
 	trigoid = InsertCmdTriggerTuple(tgrel, stmt->command, stmt->trigname,
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to