Thom Brown <thombr...@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
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f5f2079..5f84751 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,
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index a18b0bb..87d4ca3 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -297,6 +297,7 @@ get_object_name(CommandContext cmd, ObjectType objtype,
case OBJECT_CAST:
cmd->objectname = NULL;
break;
+ case OBJECT_RULE:
case OBJECT_TRIGGER:
cmd->objectname = pstrdup(strVal(llast(objname)));
break;
@@ -313,7 +314,6 @@ get_object_name(CommandContext cmd, ObjectType objtype,
case OBJECT_OPERATOR:
case OBJECT_LANGUAGE:
case OBJECT_CMDTRIGGER:
- case OBJECT_RULE:
case OBJECT_FDW:
case OBJECT_FOREIGN_SERVER:
case OBJECT_OPCLASS:
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index a87feca..ce97b8f 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1399,6 +1399,19 @@ AlterFunction(AlterFunctionStmt *stmt)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("ROWS is not applicable when function does not return a set")));
}
+
+ /* Call BEFORE ALTER FUNCTION command triggers */
+ InitCommandContext(&cmd, (Node *)stmt);
+
+ if (CommandFiresTriggers(&cmd))
+ {
+ cmd.objectId = HeapTupleGetOid(tup);
+ cmd.objectname = pstrdup(NameStr(procForm->proname));
+ cmd.schemaname = get_namespace_name(procForm->pronamespace);
+
+ ExecBeforeCommandTriggers(&cmd);
+ }
+
if (set_items)
{
Datum datum;
@@ -1434,18 +1447,6 @@ AlterFunction(AlterFunctionStmt *stmt)
repl_val, repl_null, repl_repl);
}
- /* Call BEFORE ALTER FUNCTION command triggers */
- InitCommandContext(&cmd, (Node *)stmt);
-
- if (CommandFiresTriggers(&cmd))
- {
- cmd.objectId = InvalidOid;
- cmd.objectname = pstrdup(NameStr(procForm->proname));
- cmd.schemaname = get_namespace_name(procForm->pronamespace);
-
- ExecBeforeCommandTriggers(&cmd);
- }
-
/* Do the update */
simple_heap_update(rel, &tup->t_self, tup);
CatalogUpdateIndexes(rel, tup);
diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index c8dd6ce..f8c0c66 100644
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -869,7 +869,7 @@ AlterOpFamily(AlterOpFamilyStmt *stmt)
opfForm = (Form_pg_opfamily) GETSTRUCT(htup);
cmd.objectId = opfamilyoid;
- cmd.objectname = NameStr(opfForm->opfname);
+ cmd.objectname = pstrdup(NameStr(opfForm->opfname));
cmd.schemaname = get_namespace_name(opfForm->opfnamespace);
ReleaseSysCache(htup);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 90b3525..f7f9a0e 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -437,7 +437,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
{
cmd.objectId = InvalidOid;
cmd.objectname = stmt->trigname;
- cmd.schemaname = get_namespace_name(RelationGetNamespace(rel));
+ cmd.schemaname = NULL; /* triggers are not schema qualified */
ExecBeforeCommandTriggers(&cmd);
}
diff --git a/src/backend/commands/tsearchcmds.c b/src/backend/commands/tsearchcmds.c
index 7f473ca..fa69430 100644
--- a/src/backend/commands/tsearchcmds.c
+++ b/src/backend/commands/tsearchcmds.c
@@ -857,7 +857,7 @@ AlterTSDictionary(AlterTSDictionaryStmt *stmt)
{
Form_pg_ts_dict form = (Form_pg_ts_dict) GETSTRUCT(tup);
cmd.objectId = dictId;
- cmd.objectname = NameStr(form->dictname);
+ cmd.objectname = pstrdup(NameStr(form->dictname));
cmd.schemaname = get_namespace_name(form->dictnamespace);
ExecBeforeCommandTriggers(&cmd);
@@ -1858,7 +1858,7 @@ AlterTSConfiguration(AlterTSConfigurationStmt *stmt)
Form_pg_ts_config form = (Form_pg_ts_config) GETSTRUCT(tup);
cmd.objectId = HeapTupleGetOid(tup);
- cmd.objectname = NameStr(form->cfgname);
+ cmd.objectname = pstrdup(NameStr(form->cfgname));
cmd.schemaname = get_namespace_name(form->cfgnamespace);
ExecBeforeCommandTriggers(&cmd);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 958367c..399f2e0 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -492,8 +492,7 @@ DefineQueryRewrite(char *rulename,
{
cmd->objectId = InvalidOid;
cmd->objectname = rulename;
- cmd->schemaname =
- get_namespace_name(RelationGetNamespace(event_relation));
+ cmd->schemaname = NULL; /* rules are not schema qualified */
ExecBeforeCommandTriggers(cmd);
}
diff --git a/src/test/regress/expected/cmdtriggers.out b/src/test/regress/expected/cmdtriggers.out
index ee01e41..4c3fae8 100644
--- a/src/test/regress/expected/cmdtriggers.out
+++ b/src/test/regress/expected/cmdtriggers.out
@@ -449,9 +449,9 @@ NOTICE: snitch: BEFORE any CREATE FUNCTION cmd.trigfunc
NOTICE: snitch: AFTER CREATE FUNCTION cmd.trigfunc
NOTICE: snitch: AFTER any CREATE FUNCTION cmd.trigfunc
create trigger footg before update on cmd.foo for each row execute procedure cmd.trigfunc();
-NOTICE: snitch: BEFORE any CREATE TRIGGER cmd.footg
-NOTICE: snitch: BEFORE CREATE TRIGGER cmd.footg
-NOTICE: snitch: AFTER any CREATE TRIGGER cmd.footg
+NOTICE: snitch: BEFORE any CREATE TRIGGER <NULL>.footg
+NOTICE: snitch: BEFORE CREATE TRIGGER <NULL>.footg
+NOTICE: snitch: AFTER any CREATE TRIGGER <NULL>.footg
alter trigger footg on cmd.foo rename to foo_trigger;
NOTICE: snitch: BEFORE any ALTER TRIGGER cmd.footg
NOTICE: snitch: BEFORE ALTER TRIGGER cmd.footg
@@ -494,6 +494,9 @@ create text search dictionary test_stem (
NOTICE: snitch: BEFORE any CREATE TEXT SEARCH DICTIONARY public.test_stem
NOTICE: snitch: AFTER CREATE TEXT SEARCH DICTIONARY public.test_stem
NOTICE: snitch: AFTER any CREATE TEXT SEARCH DICTIONARY public.test_stem
+alter text search dictionary test_stem (StopWords = dutch );
+NOTICE: snitch: BEFORE any ALTER TEXT SEARCH DICTIONARY public.test_stem
+NOTICE: snitch: AFTER any ALTER TEXT SEARCH DICTIONARY public.test_stem
create text search parser test_parser (
start = prsd_start,
gettoken = prsd_nexttoken,
diff --git a/src/test/regress/sql/cmdtriggers.sql b/src/test/regress/sql/cmdtriggers.sql
index 82abb23..75bc573 100644
--- a/src/test/regress/sql/cmdtriggers.sql
+++ b/src/test/regress/sql/cmdtriggers.sql
@@ -218,6 +218,7 @@ create text search dictionary test_stem (
template = snowball,
language = 'english', stopwords = 'english'
);
+alter text search dictionary test_stem (StopWords = dutch );
create text search parser test_parser (
start = prsd_start,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
