8000 Ensure that BEFORE STATEMENT triggers fire the right number of times. · postgres/postgres@fd31f9f · GitHub
[go: up one dir, main page]
Skip to content

Commit fd31f9f

Browse files
tglsfdctglsfdc
committed
Ensure that BEFORE STATEMENT triggers fire the right number of times.
Commit 0f79440 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
1 parent cad2207 commit fd31f9f

File tree

4 files changed

+87
-17
lines changed

4 files changed

+87
-17
lines changed

‎doc/src/sgml/ref/create_trigger.sgml

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable>
250250
One of <literal>INSERT</literal>, <literal>UPDATE</literal>,
251251
<literal>DELETE</literal>, or <literal>TRUNCATE</literal>;
252252
this specifies the event that will fire the trigger. Multiple
253-
events can be specified using <literal>OR</literal>.
253+
events can be specified using <literal>OR</literal>, except when
254+
transition relations are requested.
254255
</para>
255256

256257
<para>
@@ -263,7 +264,10 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
263264
is mentioned as a target of the <command>UPDATE</> command.
264265
</para>
265266

266-
<para><literal>INSTEAD OF UPDATE</> events do not support lists of columns.
267+
<para>
268+
<literal>INSTEAD OF UPDATE</> events do not allow a list of columns.
269+
A column list cannot be specified when requesting transition relations,
270+
either.
267271
</para>
268272
</listitem>
269273
</varlistentry>
@@ -490,9 +494,9 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
490494
trigger that requests transition relations will cause the foreign-key
491495
enforcement actions triggered by a single SQL command to be split into
492496
multiple steps, each with its own transition relation(s). In such cases,
493-
any <literal>AFTER STATEMENT</> triggers that are present will be fired
494-
once per creation of a transition relation, ensuring that the triggers see
495-
each affected row once and only once.
497+
any statement-level triggers that are present will be fired once per
498+
creation of a transition relation set, ensuring that the triggers see
499+
each affected row in a transition relation once and only once.
496500
</para>
497501

498502
<para>

‎src/backend/commands/trigger.c

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
100100
List *recheckIndexes, Bitmapset *modifiedCols,
101101
TransitionCaptureState *transition_capture);
102102
static void AfterTriggerEnlargeQueryState(void);
103+
static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType);
103104

104105

105106
/*
@@ -2229,6 +2230,11 @@ ExecBSInsertTriggers(EState *estate, ResultRelInfo *relinfo)
22292230
if (!trigdesc->trig_insert_before_statement)
22302231
return;
22312232

2233+
/* no-op if we already fired BS triggers in this context */
2234+
if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc),
2235+
CMD_INSERT))
2236+
return;
2237+
22322238
LocTriggerData.type = T_TriggerData;
22332239
LocTriggerData.tg_event = TRIGGER_EVENT_INSERT |
22342240
TRIGGER_EVENT_BEFORE;
@@ -2439,6 +2445,11 @@ ExecBSDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
24392445
if (!trigdesc->trig_delete_before_statement)
24402446
return;
24412447

2448+
/* no-op if we already fired BS triggers in this context */
2449+
if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc),
2450+
CMD_DELETE))
2451+
return;
2452+
24422453
LocTriggerData.type = T_TriggerData;
24432454
LocTriggerData.tg_event = TRIGGER_EVENT_DELETE |
24442455
TRIGGER_EVENT_BEFORE;
@@ -2651,6 +2662,11 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
26512662
if (!trigdesc->trig_update_before_statement)
26522663
return;
26532664

2665+
/* no-op if we already fired BS triggers in this context */
2666+
if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc),
2667+
CMD_UPDATE))
2668+
return;
2669+
26542670
updatedCols = GetUpdatedColumns(relinfo, estate);
26552671

26562672
LocTriggerData.type = T_TriggerData;
@@ -3523,11 +3539,11 @@ typedef struct AfterTriggerEventList
35233539
*
35243540
* We create an AfterTriggersTableData struct for each target table of the
35253541
* current query, and each operation mode (INSERT/UPDATE/DELETE), that has
3526-
* either transition tables or AFTER STATEMENT triggers. This is used to
3542+
* either transition tables or statement-level triggers. This is used to
35273543
* hold the relevant transition tables, as well as info tracking whether
3528-
* we already queued the AFTER STATEMENT triggers. (We use that info to
3529-
* prevent, as much as possible, firing the same AFTER STATEMENT trigger
3530-
* more than once per statement.) These structs, along with the transition
3544+
* we already queued the statement triggers. (We use that info to prevent
3545+
* firing the same statement triggers more than once per statement, or really
3546+
* once per transition table set.) These structs, along with the transition
35313547
* table tuplestores, live in the (sub)transaction's CurTransactionContext.
35323548
* That's sufficient lifespan because we don't allow transition tables to be
35333549
* used by deferrable triggers, so they only need to survive until
@@ -3576,8 +3592,9 @@ struct AfterTriggersTableData
35763592
Oid relid; /* target table's OID */
35773593
CmdType cmdType; /* event type, CMD_INSERT/UPDATE/DELETE */
35783594
bool closed; /* true when no longer OK to add tuples */
3579-
bool stmt_trig_done; /* did we already queue stmt-level triggers? */
3580-
AfterTriggerEventList stmt_trig_events; /* if so, saved list pointer */
3595+
bool before_trig_done; /* did we already queue BS triggers? */
3596+
bool after_trig_done; /* did we already queue AS triggers? */
3597+
AfterTriggerEventList after_trig_events; /* if so, saved list pointer */
35813598
Tuplestorestate *old_tuplestore; /* "old" transition table, if any */
35823599
Tuplestorestate *new_tuplestore; /* "new" transition table, if any */
35833600
};
@@ -5650,6 +5667,37 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
56505667
}
56515668
}
56525669

5670+
/*
5671+
* Detect whether we already queued BEFORE STATEMENT triggers for the given
5672+
* relation + operation, and set the flag so the next call will report "true".
5673+
*/
5674+
static bool
5675+
before_stmt_triggers_fired(Oid relid, CmdType cmdType)
5676+
{
5677+
bool result;
5678+
AfterTriggersTableData *table;
5679+
5680+
/* Check state, like AfterTriggerSaveEvent. */
5681+
if (afterTriggers.query_depth < 0)
5682+
elog(ERROR, "before_stmt_triggers_fired() called outside of query");
5683+
5684+
/* Be sure we have enough space to record events at this query depth. */
5685+
if (afterTriggers.query_depth >= afterTriggers.maxquerydepth)
5686+
AfterTriggerEnlargeQueryState();
5687+
5688+
/*
5689+
* We keep this state in the AfterTriggersTableData that also holds
5690+
* transition tables for the relation + operation. In this way, if we are
5691+
* forced to make a new set of transition tables because more tuples get
5692+
* entered after we've already fired triggers, we will allow a new set of
5693+
* statement triggers to get queued.
5694+
*/
5695+
table = GetAfterTriggersTableData(relid, cmdType);
5696+
result = table->before_trig_done;
5697+
table->before_trig_done = true;
5698+
return result;
5699+
}
5700+
56535701
/*
56545702
* If we previously queued a set of AFTER STATEMENT triggers for the given
56555703
* 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)
56845732
*/
56855733
table = GetAfterTriggersTableData(relid, cmdType);
56865734

5687-
if (table->stmt_trig_done)
5735+
if (table->after_trig_done)
56885736
{
56895737
/*
56905738
* 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)
56955743
AfterTriggerEvent event;
56965744
AfterTriggerEventChunk *chunk;
56975745

5698-
if (table->stmt_trig_events.tail)
5746+
if (table->after_trig_events.tail)
56995747
{
5700-
chunk = table->stmt_trig_events.tail;
5701-
event = (AfterTriggerEvent) table->stmt_trig_events.tailfree;
5748+
chunk = table->after_trig_events.tail;
5749+
event = (AfterTriggerEvent) table->after_trig_events.tailfree;
57025750
}
57035751
else
57045752
{
@@ -5737,8 +5785,8 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent)
57375785
done:
57385786

57395787
/* In any case, save current insertion point for next time */
5740-
table->stmt_trig_done = true;
5741-
table->stmt_trig_events = qs->events;
5788+
table->after_trig_done = true;
5789+
table->after_trig_events = qs->events;
57425790
}
57435791

57445792
/*

‎src/test/regress/expected/triggers.out

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2289,6 +2289,9 @@ create table refd_table (a int primary key, b text);
22892289
create table trig_table (a int, b text,
22902290
foreign key (a) references refd_table on update cascade on delete cascade
22912291
);
2292+
create trigger trig_table_before_trig
2293+
before insert or update or delete on trig_table
2294+
for each statement execute procedure trigger_func('trig_table');
22922295
create trigger trig_table_insert_trig
22932296
after insert on trig_table referencing new table as new_table
22942297
for each statement execute procedure dump_insert();
@@ -2309,8 +2312,10 @@ insert into trig_table values
23092312
(2, 'two b'),
23102313
(3, 'three a'),
23112314
(3, 'three b');
2315+
NOTICE: trigger_func(trig_table) called: action = INSERT, when = BEFORE, level = STATEMENT
23122316
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")
23132317
update refd_table set a = 11 where b = 'one';
2318+
NOTICE: trigger_func(trig_table) called: action = UPDATE, when = BEFORE, level = STATEMENT
23142319
NOTICE: trigger = trig_table_update_trig, old table = (1,"one a"), (1,"one b"), new table = (11,"one a"), (11,"one b")
23152320
select * from trig_table;
23162321
a | b
@@ -2324,6 +2329,7 @@ select * from trig_table;
23242329
(6 rows)
23252330

23262331
delete from refd_table where length(b) = 3;
2332+
NOTICE: trigger_func(trig_table) called: action = DELETE, when = BEFORE, level = STATEMENT
23272333
NOTICE: trigger = trig_table_delete_trig, old table = (2,"two a"), (2,"two b"), (11,"one a"), (11,"one b")
23282334
select * from trig_table;
23292335
a | b
@@ -2338,6 +2344,9 @@ drop table refd_table, trig_table;
23382344
--
23392345
create table self_ref (a int primary key,
23402346
b int references self_ref(a) on delete cascade);
2347+
create trigger self_ref_before_trig
2348+
before delete on self_ref
2349+
for each statement execute procedure trigger_func('self_ref');
23412350
create trigger self_ref_r_trig
23422351
after delete on self_ref referencing old table as old_table
23432352
for each row execute procedure dump_delete();
@@ -2346,7 +2355,9 @@ create trigger self_ref_s_trig
23462355
for each statement execute procedure dump_delete();
23472356
insert into self_ref values (1, null), (2, 1), (3, 2);
23482357
delete from self_ref where a = 1;
2358+
NOTICE: trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT
23492359
NOTICE: trigger = self_ref_r_trig, old table = (1,), (2,1)
2360+
NOTICE: trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT
23502361
NOTICE: trigger = self_ref_r_trig, old table = (1,), (2,1)
23512362
NOTICE: trigger = self_ref_s_trig, old table = (1,), (2,1)
23522363
NOTICE: trigger = self_ref_r_trig, old table = (3,2)
@@ -2355,6 +2366,7 @@ NOTICE: trigger = self_ref_s_trig, old table = (3,2)
23552366
drop trigger self_ref_r_trig on self_ref;
23562367
insert into self_ref values (1, null), (2, 1), (3, 2), (4, 3);
23572368
delete from self_ref where a = 1;
2369+
NOTICE: trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT
23582370
NOTICE: trigger = self_ref_s_trig, old table = (1,), (2,1), (3,2), (4,3)
23592371
drop table self_ref;
23602372
-- cleanup

‎src/test/regress/sql/triggers.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1795,6 +1795,9 @@ create table trig_table (a int, b text,
17951795
foreign key (a) references refd_table on update cascade on delete cascade
17961796
);
17971797

1798+
create trigger trig_table_before_trig
1799+
before insert or update or delete on trig_table
1800+
for each statement execute procedure trigger_func('trig_table');
17981801
create trigger trig_table_insert_trig
17991802
after insert on trig_table referencing new table as new_table
18001803
for each statement execute procedure dump_insert();
@@ -1834,6 +1837,9 @@ drop table refd_table, trig_table;
18341837
create table self_ref (a int primary key,
18351838
b int references self_ref(a) on delete cascade);
18361839

1840+
create trigger self_ref_before_trig
1841+
before delete on self_ref
1842+
for each statement execute procedure trigger_func('self_ref');
18371843
create trigger self_ref_r_trig
18381844
after delete on self_ref referencing old table as old_table
18391845
for each row execute procedure dump_delete();

0 commit comments

Comments
 (0)
0