From 92bf7e2d027466d750b4ac5b026f6f4ac29be881 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 14 Nov 2020 17:05:34 -0500 Subject: [PATCH] Provide the OR REPLACE option for CREATE TRIGGER. This is mostly straightforward. However, we disallow replacing constraint triggers or changing the is-constraint property; perhaps that can be added later, but the complexity versus benefit tradeoff doesn't look very good. Also, no special thought is taken here for whether replacing an existing trigger should result in changes to queued-but-not-fired trigger actions. We just document that if you're surprised by the results, too bad, don't do that. (Note that any such pending trigger activity would have to be within the current session.) Takamichi Osumi, reviewed at various times by Surafel Temesgen, Peter Smith, and myself Discussion: https://postgr.es/m/0DDF369B45A1B44B8A687ED43F06557C010BC362@G01JPEXMBYT03 --- doc/src/sgml/ref/create_trigger.sgml | 76 +++++++---- src/backend/catalog/index.c | 7 +- src/backend/commands/tablecmds.c | 28 ++-- src/backend/commands/trigger.c | 174 ++++++++++++++++++------- src/backend/nodes/copyfuncs.c | 3 +- src/backend/nodes/equalfuncs.c | 3 +- src/backend/parser/gram.y | 52 ++++---- src/include/nodes/parsenodes.h | 3 +- src/test/regress/expected/triggers.out | 89 +++++++++++++ src/test/regress/sql/triggers.sql | 87 +++++++++++++ 10 files changed, 408 insertions(+), 114 deletions(-) diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index 60346e1e83..561af989a4 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -26,7 +26,7 @@ PostgreSQL documentation -CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } { event [ OR ... ] } +CREATE [ OR REPLACE ] [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } { event [ OR ... ] } ON table_name [ FROM referenced_table_name ] [ NOT DEFERRABLE | [ DEFERRABLE ] [ INITIALLY IMMEDIATE | INITIALLY DEFERRED ] ] @@ -48,13 +48,21 @@ CREATE [ CONSTRAINT ] TRIGGER name Description - CREATE TRIGGER creates a new trigger. The + CREATE TRIGGER creates a new trigger. + CREATE OR REPLACE TRIGGER will either create a + new trigger, or replace an existing trigger. The trigger will be associated with the specified table, view, or foreign table and will execute the specified function function_name when certain operations are performed on that table. + + To replace the current definition of an existing trigger, use + CREATE OR REPLACE TRIGGER, specifying the existing + trigger's name and parent table. All other properties are replaced. + + The trigger can be specified to fire before the operation is attempted on a row (before constraints are checked and @@ -436,7 +444,7 @@ UPDATE OF column_name1 [, column_name2Notes - To create a trigger on a table, the user must have the + To create or replace a trigger on a table, the user must have the TRIGGER privilege on the table. The user must also have EXECUTE privilege on the trigger function. @@ -445,6 +453,17 @@ UPDATE OF column_name1 [, column_name2DROP TRIGGER to remove a trigger. + + Creating a row-level trigger on a partitioned table will cause an + identical clone trigger to be created on each of its + existing partitions; and any partitions created or attached later will have + an identical trigger, too. If there is a conflictingly-named trigger on a + child partition already, an error occurs unless CREATE OR REPLACE + TRIGGER is used, in which case that trigger is replaced with a + clone trigger. When a partition is detached from its parent, its clone + triggers are removed. + + A column-specific trigger (one defined using the UPDATE OF column_name syntax) will fire when any @@ -457,12 +476,6 @@ UPDATE OF column_name1 [, column_name2 - - There are a few built-in trigger functions that can be used to - solve common problems without having to write your own trigger code; - see . - - In a BEFORE trigger, the WHEN condition is evaluated just before the function is or would be executed, so using @@ -528,14 +541,6 @@ UPDATE OF column_name1 [, column_name2 - - Creating a row-level trigger on a partitioned table will cause identical - triggers to be created in all its existing partitions; and any partitions - created or attached later will contain an identical trigger, too. - If the partition is detached from its parent, the trigger is removed. - Triggers on partitioned tables may not be INSTEAD OF. - - Modifying a partitioned table or a table with inheritance children fires statement-level triggers attached to the explicitly named table, but not @@ -546,9 +551,32 @@ UPDATE OF column_name1 [, column_name2REFERENCING clause, then before and after images of rows are visible from all affected partitions or child tables. In the case of inheritance children, the row images include only columns - that are present in the table that the trigger is attached to. Currently, - row-level triggers with transition relations cannot be defined on - partitions or inheritance child tables. + that are present in the table that the trigger is attached to. + + + + Currently, row-level triggers with transition relations cannot be defined + on partitions or inheritance child tables. Also, triggers on partitioned + tables may not be INSTEAD OF. + + + + Currently, the OR REPLACE option is not supported for + constraint triggers. + + + + Replacing an existing trigger within a transaction that has already + performed updating actions on the trigger's table is not recommended. + Trigger firing decisions, or portions of firing decisions, that have + already been made will not be reconsidered, so the effects could be + surprising. + + + + There are a few built-in trigger functions that can be used to + solve common problems without having to write your own trigger code; + see . @@ -566,11 +594,12 @@ CREATE TRIGGER check_update EXECUTE FUNCTION check_account_update(); - The same, but only execute the function if column balance - is specified as a target in the UPDATE command: + Modify that trigger definition to only execute the function if + column balance is specified as a target in + the UPDATE command: -CREATE TRIGGER check_update +CREATE OR REPLACE TRIGGER check_update BEFORE UPDATE OF balance ON accounts FOR EACH ROW EXECUTE FUNCTION check_account_update(); @@ -728,6 +757,7 @@ CREATE TRIGGER paired_items_update CREATE CONSTRAINT TRIGGER is a PostgreSQL extension of the SQL standard. + So is the OR REPLACE option. diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index b88b4a1f12..731610c701 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2093,9 +2093,10 @@ index_constraint_create(Relation heapRelation, */ if (deferrable) { - CreateTrigStmt *trigger; + CreateTrigStmt *trigger = makeNode(CreateTrigStmt); - trigger = makeNode(CreateTrigStmt); + trigger->replace = false; + trigger->isconstraint = true; trigger->trigname = (constraintType == CONSTRAINT_PRIMARY) ? "PK_ConstraintTrigger" : "Unique_ConstraintTrigger"; @@ -2107,7 +2108,7 @@ index_constraint_create(Relation heapRelation, trigger->events = TRIGGER_TYPE_INSERT | TRIGGER_TYPE_UPDATE; trigger->columns = NIL; trigger->whenClause = NULL; - trigger->isconstraint = true; + trigger->transitionRels = NIL; trigger->deferrable = true; trigger->initdeferred = initdeferred; trigger->constrrel = NULL; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e3cfaf8b07..46f1637e77 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10550,10 +10550,10 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, * and "RI_ConstraintTrigger_c_NNNN" for the check triggers. */ fk_trigger = makeNode(CreateTrigStmt); + fk_trigger->replace = false; + fk_trigger->isconstraint = true; fk_trigger->trigname = "RI_ConstraintTrigger_c"; fk_trigger->relation = NULL; - fk_trigger->row = true; - fk_trigger->timing = TRIGGER_TYPE_AFTER; /* Either ON INSERT or ON UPDATE */ if (on_insert) @@ -10567,14 +10567,15 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, fk_trigger->events = TRIGGER_TYPE_UPDATE; } + fk_trigger->args = NIL; + fk_trigger->row = true; + fk_trigger->timing = TRIGGER_TYPE_AFTER; fk_trigger->columns = NIL; - fk_trigger->transitionRels = NIL; fk_trigger->whenClause = NULL; - fk_trigger->isconstraint = true; + fk_trigger->transitionRels = NIL; fk_trigger->deferrable = fkconstraint->deferrable; fk_trigger->initdeferred = fkconstraint->initdeferred; fk_trigger->constrrel = NULL; - fk_trigger->args = NIL; (void) CreateTrigger(fk_trigger, NULL, myRelOid, refRelOid, constraintOid, indexOid, InvalidOid, InvalidOid, NULL, true, false); @@ -10599,15 +10600,17 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr * DELETE action on the referenced table. */ fk_trigger = makeNode(CreateTrigStmt); + fk_trigger->replace = false; + fk_trigger->isconstraint = true; fk_trigger->trigname = "RI_ConstraintTrigger_a"; fk_trigger->relation = NULL; + fk_trigger->args = NIL; fk_trigger->row = true; fk_trigger->timing = TRIGGER_TYPE_AFTER; fk_trigger->events = TRIGGER_TYPE_DELETE; fk_trigger->columns = NIL; - fk_trigger->transitionRels = NIL; fk_trigger->whenClause = NULL; - fk_trigger->isconstraint = true; + fk_trigger->transitionRels = NIL; fk_trigger->constrrel = NULL; switch (fkconstraint->fk_del_action) { @@ -10641,7 +10644,6 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr (int) fkconstraint->fk_del_action); break; } - fk_trigger->args = NIL; (void) CreateTrigger(fk_trigger, NULL, refRelOid, RelationGetRelid(rel), constraintOid, @@ -10655,15 +10657,17 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr * UPDATE action on the referenced table. */ fk_trigger = makeNode(CreateTrigStmt); + fk_trigger->replace = false; + fk_trigger->isconstraint = true; fk_trigger->trigname = "RI_ConstraintTrigger_a"; fk_trigger->relation = NULL; + fk_trigger->args = NIL; fk_trigger->row = true; fk_trigger->timing = TRIGGER_TYPE_AFTER; fk_trigger->events = TRIGGER_TYPE_UPDATE; fk_trigger->columns = NIL; - fk_trigger->transitionRels = NIL; fk_trigger->whenClause = NULL; - fk_trigger->isconstraint = true; + fk_trigger->transitionRels = NIL; fk_trigger->constrrel = NULL; switch (fkconstraint->fk_upd_action) { @@ -10697,7 +10701,6 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr (int) fkconstraint->fk_upd_action); break; } - fk_trigger->args = NIL; (void) CreateTrigger(fk_trigger, NULL, refRelOid, RelationGetRelid(rel), constraintOid, @@ -16898,6 +16901,8 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) } trigStmt = makeNode(CreateTrigStmt); + trigStmt->replace = false; + trigStmt->isconstraint = OidIsValid(trigForm->tgconstraint); trigStmt->trigname = NameStr(trigForm->tgname); trigStmt->relation = NULL; trigStmt->funcname = NULL; /* passed separately */ @@ -16907,7 +16912,6 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) trigStmt->events = trigForm->tgtype & TRIGGER_TYPE_EVENT_MASK; trigStmt->columns = cols; trigStmt->whenClause = NULL; /* passed separately */ - trigStmt->isconstraint = OidIsValid(trigForm->tgconstraint); trigStmt->transitionRels = NIL; /* not supported at present */ trigStmt->deferrable = trigForm->tgdeferrable; trigStmt->initdeferred = trigForm->tginitdeferred; diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index e1f3472eca..c336b238aa 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -152,7 +152,9 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); * * When called on partitioned tables, this function recurses to create the * trigger on all the partitions, except if isInternal is true, in which - * case caller is expected to execute recursion on its own. + * case caller is expected to execute recursion on its own. in_partition + * indicates such a recursive call; outside callers should pass "false" + * (but see CloneRowTriggersToPartition). */ ObjectAddress CreateTrigger(CreateTrigStmt *stmt, const char *queryString, @@ -171,12 +173,10 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, Relation rel; AclResult aclresult; Relation tgrel; - SysScanDesc tgscan; - ScanKeyData key; Relation pgrel; - HeapTuple tuple; + HeapTuple tuple = NULL; Oid funcrettype; - Oid trigoid; + Oid trigoid = InvalidOid; char internaltrigname[NAMEDATALEN]; char *trigname; Oid constrrelid = InvalidOid; @@ -185,6 +185,9 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, char *oldtablename = NULL; char *newtablename = NULL; bool partition_recurse; + bool trigger_exists = false; + Oid existing_constraint_oid = InvalidOid; + bool existing_isInternal = false; if (OidIsValid(relOid)) rel = table_open(relOid, ShareRowExclusiveLock); @@ -688,6 +691,100 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, errmsg("function %s must return type %s", NameListToString(stmt->funcname), "trigger"))); + /* + * Scan pg_trigger to see if there is already a trigger of the same name. + * Skip this for internally generated triggers, since we'll modify the + * name to be unique below. + * + * NOTE that this is cool only because we have ShareRowExclusiveLock on + * the relation, so the trigger set won't be changing underneath us. + */ + tgrel = table_open(TriggerRelationId, RowExclusiveLock); + if (!isInternal) + { + ScanKeyData skeys[2]; + SysScanDesc tgscan; + + ScanKeyInit(&skeys[0], + Anum_pg_trigger_tgrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + + ScanKeyInit(&skeys[1], + Anum_pg_trigger_tgname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(stmt->trigname)); + + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, + NULL, 2, skeys); + + /* There should be at most one matching tuple */ + if (HeapTupleIsValid(tuple = systable_getnext(tgscan))) + { + Form_pg_trigger oldtrigger = (Form_pg_trigger) GETSTRUCT(tuple); + + trigoid = oldtrigger->oid; + existing_constraint_oid = oldtrigger->tgconstraint; + existing_isInternal = oldtrigger->tgisinternal; + trigger_exists = true; + /* copy the tuple to use in CatalogTupleUpdate() */ + tuple = heap_copytuple(tuple); + } + systable_endscan(tgscan); + } + + if (!trigger_exists) + { + /* Generate the OID for the new trigger. */ + trigoid = GetNewOidWithIndex(tgrel, TriggerOidIndexId, + Anum_pg_trigger_oid); + } + else + { + /* + * If OR REPLACE was specified, we'll replace the old trigger; + * otherwise complain about the duplicate name. + */ + if (!stmt->replace) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("trigger \"%s\" for relation \"%s\" already exists", + stmt->trigname, RelationGetRelationName(rel)))); + + /* + * An internal trigger cannot be replaced by a user-defined trigger. + * However, skip this test when in_partition, because then we're + * recursing from a partitioned table and the check was made at the + * parent level. Child triggers will always be marked "internal" (so + * this test does protect us from the user trying to replace a child + * trigger directly). + */ + if (existing_isInternal && !isInternal && !in_partition) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("trigger \"%s\" for relation \"%s\" is an internal trigger", + stmt->trigname, RelationGetRelationName(rel)))); + + /* + * It is not allowed to replace with a constraint trigger; gram.y + * should have enforced this already. + */ + Assert(!stmt->isconstraint); + + /* + * It is not allowed to replace an existing constraint trigger, + * either. (The reason for these restrictions is partly that it seems + * difficult to deal with pending trigger events in such cases, and + * partly that the command might imply changing the constraint's + * properties as well, which doesn't seem nice.) + */ + if (OidIsValid(existing_constraint_oid)) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("trigger \"%s\" for relation \"%s\" is a constraint trigger", + stmt->trigname, RelationGetRelationName(rel)))); + } + /* * If it's a user-entered CREATE CONSTRAINT TRIGGER command, make a * corresponding pg_constraint entry. @@ -727,15 +824,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, isInternal); /* is_internal */ } - /* - * Generate the trigger's OID now, so that we can use it in the name if - * needed. - */ - tgrel = table_open(TriggerRelationId, RowExclusiveLock); - - trigoid = GetNewOidWithIndex(tgrel, TriggerOidIndexId, - Anum_pg_trigger_oid); - /* * If trigger is internally generated, modify the provided trigger name to * ensure uniqueness by appending the trigger OID. (Callers will usually @@ -753,37 +841,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, trigname = stmt->trigname; } - /* - * Scan pg_trigger for existing triggers on relation. We do this only to - * give a nice error message if there's already a trigger of the same - * name. (The unique index on tgrelid/tgname would complain anyway.) We - * can skip this for internally generated triggers, since the name - * modification above should be sufficient. - * - * NOTE that this is cool only because we have ShareRowExclusiveLock on - * the relation, so the trigger set won't be changing underneath us. - */ - if (!isInternal) - { - ScanKeyInit(&key, - Anum_pg_trigger_tgrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(rel))); - tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, - NULL, 1, &key); - while (HeapTupleIsValid(tuple = systable_getnext(tgscan))) - { - Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(tuple); - - if (namestrcmp(&(pg_trigger->tgname), trigname) == 0) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("trigger \"%s\" for relation \"%s\" already exists", - trigname, RelationGetRelationName(rel)))); - } - systable_endscan(tgscan); - } - /* * Build the new pg_trigger tuple. * @@ -910,14 +967,24 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, else nulls[Anum_pg_trigger_tgnewtable - 1] = true; - tuple = heap_form_tuple(tgrel->rd_att, values, nulls); - /* - * Insert tuple into pg_trigger. + * Insert or replace tuple in pg_trigger. */ - CatalogTupleInsert(tgrel, tuple); + if (!trigger_exists) + { + tuple = heap_form_tuple(tgrel->rd_att, values, nulls); + CatalogTupleInsert(tgrel, tuple); + } + else + { + HeapTuple newtup; - heap_freetuple(tuple); + newtup = heap_form_tuple(tgrel->rd_att, values, nulls); + CatalogTupleUpdate(tgrel, &tuple->t_self, newtup); + heap_freetuple(newtup); + } + + heap_freetuple(tuple); /* free either original or new tuple */ table_close(tgrel, RowExclusiveLock); pfree(DatumGetPointer(values[Anum_pg_trigger_tgname - 1])); @@ -952,6 +1019,13 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, heap_freetuple(tuple); table_close(pgrel, RowExclusiveLock); + /* + * If we're replacing a trigger, flush all the old dependencies before + * recording new ones. + */ + if (trigger_exists) + deleteDependencyRecordsFor(TriggerRelationId, trigoid, true); + /* * Record dependencies for trigger. Always place a normal dependency on * the function. diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 3031c52991..5a591d0a75 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4304,6 +4304,8 @@ _copyCreateTrigStmt(const CreateTrigStmt *from) { CreateTrigStmt *newnode = makeNode(CreateTrigStmt); + COPY_SCALAR_FIELD(replace); + COPY_SCALAR_FIELD(isconstraint); COPY_STRING_FIELD(trigname); COPY_NODE_FIELD(relation); COPY_NODE_FIELD(funcname); @@ -4313,7 +4315,6 @@ _copyCreateTrigStmt(const CreateTrigStmt *from) COPY_SCALAR_FIELD(events); COPY_NODE_FIELD(columns); COPY_NODE_FIELD(whenClause); - COPY_SCALAR_FIELD(isconstraint); COPY_NODE_FIELD(transitionRels); COPY_SCALAR_FIELD(deferrable); COPY_SCALAR_FIELD(initdeferred); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 9aa853748d..e2895a8985 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2011,6 +2011,8 @@ _equalCreateAmStmt(const CreateAmStmt *a, const CreateAmStmt *b) static bool _equalCreateTrigStmt(const CreateTrigStmt *a, const CreateTrigStmt *b) { + COMPARE_SCALAR_FIELD(replace); + COMPARE_SCALAR_FIELD(isconstraint); COMPARE_STRING_FIELD(trigname); COMPARE_NODE_FIELD(relation); COMPARE_NODE_FIELD(funcname); @@ -2020,7 +2022,6 @@ _equalCreateTrigStmt(const CreateTrigStmt *a, const CreateTrigStmt *b) COMPARE_SCALAR_FIELD(events); COMPARE_NODE_FIELD(columns); COMPARE_NODE_FIELD(whenClause); - COMPARE_SCALAR_FIELD(isconstraint); COMPARE_NODE_FIELD(transitionRels); COMPARE_SCALAR_FIELD(deferrable); COMPARE_SCALAR_FIELD(initdeferred); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 051f1f1d49..6ca9072684 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -5218,48 +5218,54 @@ am_type: *****************************************************************************/ CreateTrigStmt: - CREATE TRIGGER name TriggerActionTime TriggerEvents ON + CREATE opt_or_replace TRIGGER name TriggerActionTime TriggerEvents ON qualified_name TriggerReferencing TriggerForSpec TriggerWhen EXECUTE FUNCTION_or_PROCEDURE func_name '(' TriggerFuncArgs ')' { CreateTrigStmt *n = makeNode(CreateTrigStmt); - n->trigname = $3; - n->relation = $7; - n->funcname = $13; - n->args = $15; - n->row = $9; - n->timing = $4; - n->events = intVal(linitial($5)); - n->columns = (List *) lsecond($5); - n->whenClause = $10; - n->transitionRels = $8; - n->isconstraint = false; + n->replace = $2; + n->isconstraint = false; + n->trigname = $4; + n->relation = $8; + n->funcname = $14; + n->args = $16; + n->row = $10; + n->timing = $5; + n->events = intVal(linitial($6)); + n->columns = (List *) lsecond($6); + n->whenClause = $11; + n->transitionRels = $9; n->deferrable = false; n->initdeferred = false; n->constrrel = NULL; $$ = (Node *)n; } - | CREATE CONSTRAINT TRIGGER name AFTER TriggerEvents ON + | CREATE opt_or_replace CONSTRAINT TRIGGER name AFTER TriggerEvents ON qualified_name OptConstrFromTable ConstraintAttributeSpec FOR EACH ROW TriggerWhen EXECUTE FUNCTION_or_PROCEDURE func_name '(' TriggerFuncArgs ')' { CreateTrigStmt *n = makeNode(CreateTrigStmt); - n->trigname = $4; - n->relation = $8; - n->funcname = $17; - n->args = $19; + n->replace = $2; + if (n->replace) /* not supported, see CreateTrigger */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("CREATE OR REPLACE CONSTRAINT TRIGGER is not supported"))); + n->isconstraint = true; + n->trigname = $5; + n->relation = $9; + n->funcname = $18; + n->args = $20; n->row = true; n->timing = TRIGGER_TYPE_AFTER; - n->events = intVal(linitial($6)); - n->columns = (List *) lsecond($6); - n->whenClause = $14; + n->events = intVal(linitial($7)); + n->columns = (List *) lsecond($7); + n->whenClause = $15; n->transitionRels = NIL; - n->isconstraint = true; - processCASbits($10, @10, "TRIGGER", + processCASbits($11, @11, "TRIGGER", &n->deferrable, &n->initdeferred, NULL, NULL, yyscanner); - n->constrrel = $9; + n->constrrel = $10; $$ = (Node *)n; } ; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 7ef9b0eac0..a2dcdef3fa 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2425,6 +2425,8 @@ typedef struct CreateAmStmt typedef struct CreateTrigStmt { NodeTag type; + bool replace; /* replace trigger if already exists */ + bool isconstraint; /* This is a constraint trigger */ char *trigname; /* TRIGGER's name */ RangeVar *relation; /* relation trigger is on */ List *funcname; /* qual. name of function to call */ @@ -2436,7 +2438,6 @@ typedef struct CreateTrigStmt int16 events; /* "OR" of INSERT/UPDATE/DELETE/TRUNCATE */ List *columns; /* column names, or NIL for all columns */ Node *whenClause; /* qual expression, or NULL if none */ - bool isconstraint; /* This is a constraint trigger */ /* explicitly named transition data */ List *transitionRels; /* TriggerTransition nodes, or NIL if none */ /* The remaining fields are only used for constraint triggers */ diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 0f058d2d1e..213dff6d9e 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -3153,6 +3153,95 @@ drop table self_ref; drop function dump_insert(); drop function dump_update(); drop function dump_delete(); +-- +-- Tests for CREATE OR REPLACE TRIGGER +-- +create table my_table (id integer); +create function funcA() returns trigger as $$ +begin + raise notice 'hello from funcA'; + return null; +end; $$ language plpgsql; +create function funcB() returns trigger as $$ +begin + raise notice 'hello from funcB'; + return null; +end; $$ language plpgsql; +create trigger my_trig + after insert on my_table + for each row execute procedure funcA(); +create trigger my_trig + before insert on my_table + for each row execute procedure funcB(); -- should fail +ERROR: trigger "my_trig" for relation "my_table" already exists +insert into my_table values (1); +NOTICE: hello from funcA +create or replace trigger my_trig + before insert on my_table + for each row execute procedure funcB(); -- OK +insert into my_table values (2); -- this insert should become a no-op +NOTICE: hello from funcB +table my_table; + id +---- + 1 +(1 row) + +drop table my_table; +-- test CREATE OR REPLACE TRIGGER on partition table +create table parted_trig (a int) partition by range (a); +create table parted_trig_1 partition of parted_trig + for values from (0) to (1000) partition by range (a); +create table parted_trig_1_1 partition of parted_trig_1 for values from (0) to (100); +create table parted_trig_2 partition of parted_trig for values from (1000) to (2000); +create table default_parted_trig partition of parted_trig default; +-- test that trigger can be replaced by another one +-- at the same level of partition table +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcA(); +insert into parted_trig (a) values (50); +NOTICE: hello from funcA +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcB(); +insert into parted_trig (a) values (50); +NOTICE: hello from funcB +-- test that child trigger cannot be replaced directly +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcA(); +insert into parted_trig (a) values (50); +NOTICE: hello from funcA +create or replace trigger my_trig + after insert on parted_trig_1 + for each row execute procedure funcB(); -- should fail +ERROR: trigger "my_trig" for relation "parted_trig_1" is an internal trigger +insert into parted_trig (a) values (50); +NOTICE: hello from funcA +drop trigger my_trig on parted_trig; +insert into parted_trig (a) values (50); +-- test that user trigger can be overwritten by one defined at upper level +create trigger my_trig + after insert on parted_trig_1 + for each row execute procedure funcA(); +insert into parted_trig (a) values (50); +NOTICE: hello from funcA +create trigger my_trig + after insert on parted_trig + for each row execute procedure funcB(); -- should fail +ERROR: trigger "my_trig" for relation "parted_trig_1" already exists +insert into parted_trig (a) values (50); +NOTICE: hello from funcA +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcB(); +insert into parted_trig (a) values (50); +NOTICE: hello from funcB +-- cleanup +drop table parted_trig; +drop function funcA(); +drop function funcB(); -- Leave around some objects for other tests create table trigger_parted (a int primary key) partition by list (a); create function trigger_parted_trigfunc() returns trigger language plpgsql as diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 1d122af14e..3f4c2aa877 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -2348,6 +2348,93 @@ drop function dump_insert(); drop function dump_update(); drop function dump_delete(); +-- +-- Tests for CREATE OR REPLACE TRIGGER +-- +create table my_table (id integer); + +create function funcA() returns trigger as $$ +begin + raise notice 'hello from funcA'; + return null; +end; $$ language plpgsql; + +create function funcB() returns trigger as $$ +begin + raise notice 'hello from funcB'; + return null; +end; $$ language plpgsql; + +create trigger my_trig + after insert on my_table + for each row execute procedure funcA(); + +create trigger my_trig + before insert on my_table + for each row execute procedure funcB(); -- should fail + +insert into my_table values (1); + +create or replace trigger my_trig + before insert on my_table + for each row execute procedure funcB(); -- OK + +insert into my_table values (2); -- this insert should become a no-op + +table my_table; + +drop table my_table; + +-- test CREATE OR REPLACE TRIGGER on partition table +create table parted_trig (a int) partition by range (a); +create table parted_trig_1 partition of parted_trig + for values from (0) to (1000) partition by range (a); +create table parted_trig_1_1 partition of parted_trig_1 for values from (0) to (100); +create table parted_trig_2 partition of parted_trig for values from (1000) to (2000); +create table default_parted_trig partition of parted_trig default; + +-- test that trigger can be replaced by another one +-- at the same level of partition table +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcA(); +insert into parted_trig (a) values (50); +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcB(); +insert into parted_trig (a) values (50); + +-- test that child trigger cannot be replaced directly +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcA(); +insert into parted_trig (a) values (50); +create or replace trigger my_trig + after insert on parted_trig_1 + for each row execute procedure funcB(); -- should fail +insert into parted_trig (a) values (50); +drop trigger my_trig on parted_trig; +insert into parted_trig (a) values (50); + +-- test that user trigger can be overwritten by one defined at upper level +create trigger my_trig + after insert on parted_trig_1 + for each row execute procedure funcA(); +insert into parted_trig (a) values (50); +create trigger my_trig + after insert on parted_trig + for each row execute procedure funcB(); -- should fail +insert into parted_trig (a) values (50); +create or replace trigger my_trig + after insert on parted_trig + for each row execute procedure funcB(); +insert into parted_trig (a) values (50); + +-- cleanup +drop table parted_trig; +drop function funcA(); +drop function funcB(); + -- Leave around some objects for other tests create table trigger_parted (a int primary key) partition by list (a); create function trigger_parted_trigfunc() returns trigger language plpgsql as