Skip to content

Commit 547e454

Browse files
author
Etsuro Fujita
committed
Fix handling of multiple AFTER ROW triggers on a foreign table.
AfterTriggerExecute() retrieves a fresh tuple or pair of tuples from a tuplestore and then stores the tuple(s) in the passed-in slot(s) if AFTER_TRIGGER_FDW_FETCH, while it uses the most-recently-retrieved tuple(s) stored in the slot(s) if AFTER_TRIGGER_FDW_REUSE. This was done correctly before 12, but commit ff11e7f broke it by mistakenly clearing the tuple(s) stored in the slot(s) in that function, leading to an assertion failure as reported in bug #16139 from Alexander Lakhin. Also, fix some other issues with the aforementioned commit in passing: * For tg_newslot, which is a slot added to the TriggerData struct by the commit to store new updated tuples, it didn't ensure the slot was NULL if there was no such tuple. * The commit failed to update the documentation about the trigger interface. Author: Etsuro Fujita Backpatch-through: 12 Discussion: https://postgr.es/m/16139-94f9ccf0db6119ec%40postgresql.org
1 parent 001362c commit 547e454

File tree

4 files changed

+81
-24
lines changed

4 files changed

+81
-24
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6459,6 +6459,36 @@ DROP TRIGGER trig_row_after ON rem1;
64596459
DROP TRIGGER trig_stmt_before ON rem1;
64606460
DROP TRIGGER trig_stmt_after ON rem1;
64616461
DELETE from rem1;
6462+
-- Test multiple AFTER ROW triggers on a foreign table
6463+
CREATE TRIGGER trig_row_after1
6464+
AFTER INSERT OR UPDATE OR DELETE ON rem1
6465+
FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
6466+
CREATE TRIGGER trig_row_after2
6467+
AFTER INSERT OR UPDATE OR DELETE ON rem1
6468+
FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
6469+
insert into rem1 values(1,'insert');
6470+
NOTICE: trig_row_after1(23, skidoo) AFTER ROW INSERT ON rem1
6471+
NOTICE: NEW: (1,insert)
6472+
NOTICE: trig_row_after2(23, skidoo) AFTER ROW INSERT ON rem1
6473+
NOTICE: NEW: (1,insert)
6474+
update rem1 set f2 = 'update' where f1 = 1;
6475+
NOTICE: trig_row_after1(23, skidoo) AFTER ROW UPDATE ON rem1
6476+
NOTICE: OLD: (1,insert),NEW: (1,update)
6477+
NOTICE: trig_row_after2(23, skidoo) AFTER ROW UPDATE ON rem1
6478+
NOTICE: OLD: (1,insert),NEW: (1,update)
6479+
update rem1 set f2 = f2 || f2;
6480+
NOTICE: trig_row_after1(23, skidoo) AFTER ROW UPDATE ON rem1
6481+
NOTICE: OLD: (1,update),NEW: (1,updateupdate)
6482+
NOTICE: trig_row_after2(23, skidoo) AFTER ROW UPDATE ON rem1
6483+
NOTICE: OLD: (1,update),NEW: (1,updateupdate)
6484+
delete from rem1;
6485+
NOTICE: trig_row_after1(23, skidoo) AFTER ROW DELETE ON rem1
6486+
NOTICE: OLD: (1,updateupdate)
6487+
NOTICE: trig_row_after2(23, skidoo) AFTER ROW DELETE ON rem1
6488+
NOTICE: OLD: (1,updateupdate)
6489+
-- cleanup
6490+
DROP TRIGGER trig_row_after1 ON rem1;
6491+
DROP TRIGGER trig_row_after2 ON rem1;
64626492
-- Test WHEN conditions
64636493
CREATE TRIGGER trig_row_before_insupd
64646494
BEFORE INSERT OR UPDATE ON rem1
@@ -6689,7 +6719,7 @@ NOTICE: trig_row_after(23, skidoo) AFTER ROW INSERT ON rem1
66896719
NOTICE: NEW: (13,"test triggered !")
66906720
ctid
66916721
--------
6692-
(0,29)
6722+
(0,32)
66936723
(1 row)
66946724

66956725
-- cleanup

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,23 @@ DROP TRIGGER trig_stmt_after ON rem1;
14851485

14861486
DELETE from rem1;
14871487

1488+
-- Test multiple AFTER ROW triggers on a foreign table
1489+
CREATE TRIGGER trig_row_after1
1490+
AFTER INSERT OR UPDATE OR DELETE ON rem1
1491+
FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
1492+
1493+
CREATE TRIGGER trig_row_after2
1494+
AFTER INSERT OR UPDATE OR DELETE ON rem1
1495+
FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
1496+
1497+
insert into rem1 values(1,'insert');
1498+
update rem1 set f2 = 'update' where f1 = 1;
1499+
update rem1 set f2 = f2 || f2;
1500+
delete from rem1;
1501+
1502+
-- cleanup
1503+
DROP TRIGGER trig_row_after1 ON rem1;
1504+
DROP TRIGGER trig_row_after2 ON rem1;
14881505

14891506
-- Test WHEN conditions
14901507

‎doc/src/sgml/trigger.sgml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -511,8 +511,8 @@ typedef struct TriggerData
511511
HeapTuple tg_trigtuple;
512512
HeapTuple tg_newtuple;
513513
Trigger *tg_trigger;
514-
Buffer tg_trigtuplebuf;
515-
Buffer tg_newtuplebuf;
514+
TupleTableSlot *tg_trigslot;
515+
TupleTableSlot *tg_newslot;
516516
Tuplestorestate *tg_oldtable;
517517
Tuplestorestate *tg_newtable;
518518
} TriggerData;
@@ -714,21 +714,21 @@ typedef struct Trigger
714714
</varlistentry>
715715

716716
<varlistentry>
717-
<term><structfield>tg_trigtuplebuf</structfield></term>
717+
<term><structfield>tg_trigslot</structfield></term>
718718
<listitem>
719719
<para>
720-
The buffer containing <structfield>tg_trigtuple</structfield>, or <symbol>InvalidBuffer</symbol> if there
721-
is no such tuple or it is not stored in a disk buffer.
720+
The slot containing <structfield>tg_trigtuple</structfield>,
721+
or a <symbol>NULL</symbol> pointer if there is no such tuple.
722722
</para>
723723
</listitem>
724724
</varlistentry>
725725

726726
<varlistentry>
727-
<term><structfield>tg_newtuplebuf</structfield></term>
727+
<term><structfield>tg_newslot</structfield></term>
728728
<listitem>
729729
<para>
730-
The buffer containing <structfield>tg_newtuple</structfield>, or <symbol>InvalidBuffer</symbol> if there
731-
is no such tuple or it is not stored in a disk buffer.
730+
The slot containing <structfield>tg_newtuple</structfield>,
731+
or a <symbol>NULL</symbol> pointer if there is no such tuple.
732732
</para>
733733
</listitem>
734734
</varlistentry>

‎src/backend/commands/trigger.c

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4258,12 +4258,17 @@ AfterTriggerExecute(EState *estate,
42584258
LocTriggerData.tg_trigtuple =
42594259
ExecFetchSlotHeapTuple(trig_tuple_slot1, true, &should_free_trig);
42604260

4261-
LocTriggerData.tg_newslot = trig_tuple_slot2;
4262-
LocTriggerData.tg_newtuple =
4263-
((evtshared->ats_event & TRIGGER_EVENT_OPMASK) ==
4264-
TRIGGER_EVENT_UPDATE) ?
4265-
ExecFetchSlotHeapTuple(trig_tuple_slot2, true, &should_free_new) : NULL;
4266-
4261+
if ((evtshared->ats_event & TRIGGER_EVENT_OPMASK) ==
4262+
TRIGGER_EVENT_UPDATE)
4263+
{
4264+
LocTriggerData.tg_newslot = trig_tuple_slot2;
4265+
LocTriggerData.tg_newtuple =
4266+
ExecFetchSlotHeapTuple(trig_tuple_slot2, true, &should_free_new);
4267+
}
4268+
else
4269+
{
4270+
LocTriggerData.tg_newtuple = NULL;
4271+
}
42674272
break;
42684273

42694274
default:
@@ -4358,10 +4363,14 @@ AfterTriggerExecute(EState *estate,
43584363
if (should_free_new)
43594364
heap_freetuple(LocTriggerData.tg_newtuple);
43604365

4361-
if (LocTriggerData.tg_trigslot)
4362-
ExecClearTuple(LocTriggerData.tg_trigslot);
4363-
if (LocTriggerData.tg_newslot)
4364-
ExecClearTuple(LocTriggerData.tg_newslot);
4366+
/* don't clear slots' contents if foreign table */
4367+
if (trig_tuple_slot1 == NULL)
4368+
{
4369+
if (LocTriggerData.tg_trigslot)
4370+
ExecClearTuple(LocTriggerData.tg_trigslot);
4371+
if (LocTriggerData.tg_newslot)
4372+
ExecClearTuple(LocTriggerData.tg_newslot);
4373+
}
43654374

43664375
/*
43674376
* If doing EXPLAIN ANALYZE, stop charging time to this trigger, and count
@@ -4515,13 +4524,14 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
45154524
trigdesc = rInfo->ri_TrigDesc;
45164525
finfo = rInfo->ri_TrigFunctions;
45174526
instr = rInfo->ri_TrigInstrument;
4527+
if (slot1 != NULL)
4528+
{
4529+
ExecDropSingleTupleTableSlot(slot1);
4530+
ExecDropSingleTupleTableSlot(slot2);
4531+
slot1 = slot2 = NULL;
4532+
}
45184533
if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
45194534
{
4520-
if (slot1 != NULL)
4521-
{
4522-
ExecDropSingleTupleTableSlot(slot1);
4523-
ExecDropSingleTupleTableSlot(slot2);
4524-
}
45254535
slot1 = MakeSingleTupleTableSlot(rel->rd_att,
45264536
&TTSOpsMinimalTuple);
45274537
slot2 = MakeSingleTupleTableSlot(rel->rd_att,

0 commit comments

Comments
 (0)