From a46d5d7b984510e1d49050ff7b49be692155aab5 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 3 Dec 2015 17:07:47 -0800 Subject: [PATCH] Fix stray pointer bug in ExecOnConflictUpdate() ExecOnConflictUpdate() neglected to ensure that the ItemPointer it passed to ExecUpdate() did not point anywhere that could be modified during the execution of ExecUpdate(). The behavior of AFTER UPDATE triggers was therefore broken, as heap_update() modified the current/new ItemPointerData in the tuple in shared memory (t_ctid) before AFTER UPDATE triggers fired. NEW.* and OLD.* tuples were always spuriously identical within AFTER UPDATE triggers. To fix, follow the example of ExecInitModifyTable(), and take a deep copy. Only pass a pointer to the deep copy to ExecUpdate(). Reported by Stanislav Grozev, although this is not the exact fix that he proposed. --- src/backend/executor/nodeModifyTable.c | 6 ++++-- src/test/regress/expected/triggers.out | 8 ++++---- src/test/regress/sql/triggers.sql | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index dabaea9..348f913 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1032,6 +1032,7 @@ ExecOnConflictUpdate(ModifyTableState *mtstate, Relation relation = resultRelInfo->ri_RelationDesc; List *onConflictSetWhere = resultRelInfo->ri_onConflictSetWhere; HeapTupleData tuple; + ItemPointerData tuple_ctid; HeapUpdateFailureData hufd; LockTupleMode lockmode; HTSU_Result test; @@ -1181,9 +1182,10 @@ ExecOnConflictUpdate(ModifyTableState *mtstate, /* Project the new tuple version */ ExecProject(resultRelInfo->ri_onConflictSetProj, NULL); + /* Pass a deep copy of the tupleid. ExecUpdate() modifies the original. */ + tuple_ctid = tuple.t_data->t_ctid; /* Execute UPDATE with projection */ - *returning = ExecUpdate(&tuple.t_data->t_ctid, NULL, - mtstate->mt_conflproj, planSlot, + *returning = ExecUpdate(&tuple_ctid, 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; -- 1.9.1