>From e6b21647aac5e0f4eaf5d2c57fb30c1de8088b9f Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 9 Dec 2015 00:08:35 +0100 Subject: [PATCH] Fix AFTER UPDATE trigger behaviour in combination with ON CONFLICT and other things. Two bugs: * ctid of on-disk tuple was passed to ExecUpdate * the error check against repeatedly updating tuples via ON CONFLICT wasn't ironclad. Reported-By: Stanislav Grozev Author: Andres Freund and Peter Geoghegan Discussion: CAA78GVqy1+LisN-8DygekD_Ldfy=BJLarSpjGhytOsgkpMavfQ@mail.gmail.com Backpatch: 9.5, where ON CONFLICT was introduced --- src/backend/executor/nodeModifyTable.c | 14 ++++++++++++-- src/test/regress/expected/triggers.out | 8 ++++---- src/test/regress/sql/triggers.sql | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index dabaea9..3cad7a1 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1181,9 +1181,19 @@ ExecOnConflictUpdate(ModifyTableState *mtstate, /* Project the new tuple version */ ExecProject(resultRelInfo->ri_onConflictSetProj, NULL); + /* + * Finally recheck whether the row has been updated since the + * heap_lock_tuple() above. The ExecQual(), ExecWithCheckOptions() and, + * more likely, ExecProject() could have triggered an update of the row. + */ + if (HeapTupleSatisfiesUpdate(&tuple, estate->es_output_cid, buffer) == HeapTupleSelfUpdated) + ereport(ERROR, + (errcode(ERRCODE_CARDINALITY_VIOLATION), + errmsg("ON CONFLICT DO UPDATE command cannot affect row a second time"), + errhint("XXX: Ensure that no rows proposed for insertion within the same command have duplicate constrained values."))); + /* Execute UPDATE with projection */ - *returning = ExecUpdate(&tuple.t_data->t_ctid, NULL, - mtstate->mt_conflproj, planSlot, + *returning = ExecUpdate(&tuple.t_self, NULL, mtstate->mt_conflproj, planSlot, &mtstate->mt_epqstate, mtstate->ps.state, canSetTag); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index dd4a99b..a7bf5dc 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -1703,7 +1703,7 @@ create function upsert_after_func() $$ begin if (TG_OP = 'UPDATE') then - raise warning 'after update (old): %', new.*::text; + raise warning 'after update (old): %', old.*::text; raise warning 'after update (new): %', new.*::text; elsif (TG_OP = 'INSERT') then raise warning 'after insert (new): %', new.*::text; @@ -1724,7 +1724,7 @@ insert into upsert values(3, 'orange') on conflict (key) do update set color = ' WARNING: before insert (new): (3,orange) WARNING: before update (old): (3,"red trig modified") WARNING: before update (new): (3,"updated red trig modified") -WARNING: after update (old): (3,"updated red trig modified") +WARNING: after update (old): (3,"red trig modified") WARNING: after update (new): (3,"updated red trig modified") insert into upsert values(4, 'green') on conflict (key) do update set color = 'updated ' || upsert.color; WARNING: before insert (new): (4,green) @@ -1734,7 +1734,7 @@ insert into upsert values(5, 'purple') on conflict (key) do update set color = ' WARNING: before insert (new): (5,purple) WARNING: before update (old): (5,"green trig modified") WARNING: before update (new): (5,"updated green trig modified") -WARNING: after update (old): (5,"updated green trig modified") +WARNING: after update (old): (5,"green trig modified") WARNING: after update (new): (5,"updated green trig modified") insert into upsert values(6, 'white') on conflict (key) do update set color = 'updated ' || upsert.color; WARNING: before insert (new): (6,white) @@ -1744,7 +1744,7 @@ insert into upsert values(7, 'pink') on conflict (key) do update set color = 'up WARNING: before insert (new): (7,pink) WARNING: before update (old): (7,"white trig modified") WARNING: before update (new): (7,"updated white trig modified") -WARNING: after update (old): (7,"updated white trig modified") +WARNING: after update (old): (7,"white trig modified") WARNING: after update (new): (7,"updated white trig modified") insert into upsert values(8, 'yellow') on conflict (key) do update set color = 'updated ' || upsert.color; WARNING: before insert (new): (8,yellow) diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 9f66702..b6de1b3 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1215,7 +1215,7 @@ create function upsert_after_func() $$ begin if (TG_OP = 'UPDATE') then - raise warning 'after update (old): %', new.*::text; + raise warning 'after update (old): %', old.*::text; raise warning 'after update (new): %', new.*::text; elsif (TG_OP = 'INSERT') then raise warning 'after insert (new): %', new.*::text; -- 2.5.0.400.gff86faf.dirty