From: Tom Lane Date: Sun, 17 Sep 2017 16:16:38 +0000 (-0400) Subject: Ensure that BEFORE STATEMENT triggers fire the right number of times. X-Git-Tag: REL_11_BETA1~1578 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fd31f9f033213e2ebf00b57ef837e1828c338fc4;p=postgresql Ensure that BEFORE STATEMENT triggers fire the right number of times. Commit 0f79440fb introduced mechanism to keep AFTER STATEMENT triggers from firing more than once per statement, which was formerly possible if more than one FK enforcement action had to be applied to a given table. Add a similar mechanism for BEFORE STATEMENT triggers, so that we don't have the unexpected situation of firing BEFORE STATEMENT triggers more often than AFTER STATEMENT. As with the previous patch, back-patch to v10. Discussion: https://postgr.es/m/22315.1505584992@sss.pgh.pa.us --- diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index 065c827271..2496250bed 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -250,7 +250,8 @@ CREATE [ CONSTRAINT ] TRIGGER name One of INSERT, UPDATE, DELETE, or TRUNCATE; this specifies the event that will fire the trigger. Multiple - events can be specified using OR. + events can be specified using OR, except when + transition relations are requested. @@ -263,7 +264,10 @@ UPDATE OF column_name1 [, column_name2UPDATE command. - INSTEAD OF UPDATE events do not support lists of columns. + + INSTEAD OF UPDATE events do not allow a list of columns. + A column list cannot be specified when requesting transition relations, + either. @@ -490,9 +494,9 @@ UPDATE OF column_name1 [, column_name2AFTER STATEMENT triggers that are present will be fired - once per creation of a transition relation, ensuring that the triggers see - each affected row once and only once. + any statement-level triggers that are present will be fired once per + creation of a transition relation set, ensuring that the triggers see + each affected row in a transition relation once and only once. diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 7e391a1092..7b411c130b 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -100,6 +100,7 @@ static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, List *recheckIndexes, Bitmapset *modifiedCols, TransitionCaptureState *transition_capture); static void AfterTriggerEnlargeQueryState(void); +static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); /* @@ -2229,6 +2230,11 @@ ExecBSInsertTriggers(EState *estate, ResultRelInfo *relinfo) if (!trigdesc->trig_insert_before_statement) return; + /* no-op if we already fired BS triggers in this context */ + if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc), + CMD_INSERT)) + return; + LocTriggerData.type = T_TriggerData; LocTriggerData.tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_BEFORE; @@ -2439,6 +2445,11 @@ ExecBSDeleteTriggers(EState *estate, ResultRelInfo *relinfo) if (!trigdesc->trig_delete_before_statement) return; + /* no-op if we already fired BS triggers in this context */ + if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc), + CMD_DELETE)) + return; + LocTriggerData.type = T_TriggerData; LocTriggerData.tg_event = TRIGGER_EVENT_DELETE | TRIGGER_EVENT_BEFORE; @@ -2651,6 +2662,11 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo) if (!trigdesc->trig_update_before_statement) return; + /* no-op if we already fired BS triggers in this context */ + if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc), + CMD_UPDATE)) + return; + updatedCols = GetUpdatedColumns(relinfo, estate); LocTriggerData.type = T_TriggerData; @@ -3523,11 +3539,11 @@ typedef struct AfterTriggerEventList * * We create an AfterTriggersTableData struct for each target table of the * current query, and each operation mode (INSERT/UPDATE/DELETE), that has - * either transition tables or AFTER STATEMENT triggers. This is used to + * either transition tables or statement-level triggers. This is used to * hold the relevant transition tables, as well as info tracking whether - * we already queued the AFTER STATEMENT triggers. (We use that info to - * prevent, as much as possible, firing the same AFTER STATEMENT trigger - * more than once per statement.) These structs, along with the transition + * we already queued the statement triggers. (We use that info to prevent + * firing the same statement triggers more than once per statement, or really + * once per transition table set.) These structs, along with the transition * table tuplestores, live in the (sub)transaction's CurTransactionContext. * That's sufficient lifespan because we don't allow transition tables to be * used by deferrable triggers, so they only need to survive until @@ -3576,8 +3592,9 @@ struct AfterTriggersTableData Oid relid; /* target table's OID */ CmdType cmdType; /* event type, CMD_INSERT/UPDATE/DELETE */ bool closed; /* true when no longer OK to add tuples */ - bool stmt_trig_done; /* did we already queue stmt-level triggers? */ - AfterTriggerEventList stmt_trig_events; /* if so, saved list pointer */ + bool before_trig_done; /* did we already queue BS triggers? */ + bool after_trig_done; /* did we already queue AS triggers? */ + AfterTriggerEventList after_trig_events; /* if so, saved list pointer */ Tuplestorestate *old_tuplestore; /* "old" transition table, if any */ Tuplestorestate *new_tuplestore; /* "new" transition table, if any */ }; @@ -5650,6 +5667,37 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, } } +/* + * Detect whether we already queued BEFORE STATEMENT triggers for the given + * relation + operation, and set the flag so the next call will report "true". + */ +static bool +before_stmt_triggers_fired(Oid relid, CmdType cmdType) +{ + bool result; + AfterTriggersTableData *table; + + /* Check state, like AfterTriggerSaveEvent. */ + if (afterTriggers.query_depth < 0) + elog(ERROR, "before_stmt_triggers_fired() called outside of query"); + + /* Be sure we have enough space to record events at this query depth. */ + if (afterTriggers.query_depth >= afterTriggers.maxquerydepth) + AfterTriggerEnlargeQueryState(); + + /* + * We keep this state in the AfterTriggersTableData that also holds + * transition tables for the relation + operation. In this way, if we are + * forced to make a new set of transition tables because more tuples get + * entered after we've already fired triggers, we will allow a new set of + * statement triggers to get queued. + */ + table = GetAfterTriggersTableData(relid, cmdType); + result = table->before_trig_done; + table->before_trig_done = true; + return result; +} + /* * If we previously queued a set of AFTER STATEMENT triggers for the given * relation + operation, and they've not been fired yet, cancel them. The @@ -5684,7 +5732,7 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent) */ table = GetAfterTriggersTableData(relid, cmdType); - if (table->stmt_trig_done) + if (table->after_trig_done) { /* * We want to start scanning from the tail location that existed just @@ -5695,10 +5743,10 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent) AfterTriggerEvent event; AfterTriggerEventChunk *chunk; - if (table->stmt_trig_events.tail) + if (table->after_trig_events.tail) { - chunk = table->stmt_trig_events.tail; - event = (AfterTriggerEvent) table->stmt_trig_events.tailfree; + chunk = table->after_trig_events.tail; + event = (AfterTriggerEvent) table->after_trig_events.tailfree; } else { @@ -5737,8 +5785,8 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent) done: /* In any case, save current insertion point for next time */ - table->stmt_trig_done = true; - table->stmt_trig_events = qs->events; + table->after_trig_done = true; + table->after_trig_events = qs->events; } /* diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 3ab6be3421..85d948741e 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2289,6 +2289,9 @@ create table refd_table (a int primary key, b text); create table trig_table (a int, b text, foreign key (a) references refd_table on update cascade on delete cascade ); +create trigger trig_table_before_trig + before insert or update or delete on trig_table + for each statement execute procedure trigger_func('trig_table'); create trigger trig_table_insert_trig after insert on trig_table referencing new table as new_table for each statement execute procedure dump_insert(); @@ -2309,8 +2312,10 @@ insert into trig_table values (2, 'two b'), (3, 'three a'), (3, 'three b'); +NOTICE: trigger_func(trig_table) called: action = INSERT, when = BEFORE, level = STATEMENT NOTICE: trigger = trig_table_insert_trig, new table = (1,"one a"), (1,"one b"), (2,"two a"), (2,"two b"), (3,"three a"), (3,"three b") update refd_table set a = 11 where b = 'one'; +NOTICE: trigger_func(trig_table) called: action = UPDATE, when = BEFORE, level = STATEMENT NOTICE: trigger = trig_table_update_trig, old table = (1,"one a"), (1,"one b"), new table = (11,"one a"), (11,"one b") select * from trig_table; a | b @@ -2324,6 +2329,7 @@ select * from trig_table; (6 rows) delete from refd_table where length(b) = 3; +NOTICE: trigger_func(trig_table) called: action = DELETE, when = BEFORE, level = STATEMENT NOTICE: trigger = trig_table_delete_trig, old table = (2,"two a"), (2,"two b"), (11,"one a"), (11,"one b") select * from trig_table; a | b @@ -2338,6 +2344,9 @@ drop table refd_table, trig_table; -- create table self_ref (a int primary key, b int references self_ref(a) on delete cascade); +create trigger self_ref_before_trig + before delete on self_ref + for each statement execute procedure trigger_func('self_ref'); create trigger self_ref_r_trig after delete on self_ref referencing old table as old_table for each row execute procedure dump_delete(); @@ -2346,7 +2355,9 @@ create trigger self_ref_s_trig for each statement execute procedure dump_delete(); insert into self_ref values (1, null), (2, 1), (3, 2); delete from self_ref where a = 1; +NOTICE: trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT NOTICE: trigger = self_ref_r_trig, old table = (1,), (2,1) +NOTICE: trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT NOTICE: trigger = self_ref_r_trig, old table = (1,), (2,1) NOTICE: trigger = self_ref_s_trig, old table = (1,), (2,1) NOTICE: trigger = self_ref_r_trig, old table = (3,2) @@ -2355,6 +2366,7 @@ NOTICE: trigger = self_ref_s_trig, old table = (3,2) drop trigger self_ref_r_trig on self_ref; insert into self_ref values (1, null), (2, 1), (3, 2), (4, 3); delete from self_ref where a = 1; +NOTICE: trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT NOTICE: trigger = self_ref_s_trig, old table = (1,), (2,1), (3,2), (4,3) drop table self_ref; -- cleanup diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 30bb7d17b0..2b2236ed7d 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1795,6 +1795,9 @@ create table trig_table (a int, b text, foreign key (a) references refd_table on update cascade on delete cascade ); +create trigger trig_table_before_trig + before insert or update or delete on trig_table + for each statement execute procedure trigger_func('trig_table'); create trigger trig_table_insert_trig after insert on trig_table referencing new table as new_table for each statement execute procedure dump_insert(); @@ -1834,6 +1837,9 @@ drop table refd_table, trig_table; create table self_ref (a int primary key, b int references self_ref(a) on delete cascade); +create trigger self_ref_before_trig + before delete on self_ref + for each statement execute procedure trigger_func('self_ref'); create trigger self_ref_r_trig after delete on self_ref referencing old table as old_table for each row execute procedure dump_delete();