Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit84ac126

Browse files
committed
Fix ON CONFLICT UPDATE bug breaking AFTER UPDATE triggers.
ExecOnConflictUpdate() passed t_ctid of the to-be-updated tuple toExecUpdate(). That's problematic primarily because of two reason: Firstand foremost t_ctid could point to a different tuple. Secondly, andthat's what triggered the complaint by Stanislav, t_ctid is changed byheap_update() to point to the new tuple version. The behavior of AFTERUPDATE triggers was therefore broken, with NEW.* and OLD.* tuplesspuriously identical within AFTER UPDATE triggers.To fix both issues, pass a pointer to t_self of a on-stack HeapTupleinstead.Fixing this bug lead to one change in regression tests, which previouslyfailed due to the first issue mentioned above. There's a reasonableexpectation that test fails, as it updates one row repeatedly within oneINSERT ... ON CONFLICT statement. That is only possible if the secondupdate is triggered via ON CONFLICT ... SET, ON CONFLICT ... WHERE, orby a WITH CHECK expression, as those are executed afterExecOnConflictUpdate() does a visibility check. That could easily beprohibited, but given it's allowed for plain UPDATEs and a rare cornercase, it doesn't seem worthwhile.Reported-By: Stanislav GrozevAuthor: Andres Freund and Peter GeogheganDiscussion: CAA78GVqy1+LisN-8DygekD_Ldfy=BJLarSpjGhytOsgkpMavfQ@mail.gmail.comBackpatch: 9.5, where ON CONFLICT was introduced
1 parente3f4cfc commit84ac126

File tree

5 files changed

+25
-11
lines changed

5 files changed

+25
-11
lines changed

‎src/backend/executor/nodeModifyTable.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1181,8 +1181,17 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
11811181
/* Project the new tuple version */
11821182
ExecProject(resultRelInfo->ri_onConflictSetProj,NULL);
11831183

1184+
/*
1185+
* Note that it is possible that the target tuple has been modified in
1186+
* this session, after the above heap_lock_tuple. We choose to not error
1187+
* out in that case, in line with ExecUpdate's treatment of similar
1188+
* cases. This can happen if an UPDATE is triggered from within
1189+
* ExecQual(), ExecWithCheckOptions() or ExecProject() above, e.g. by
1190+
* selecting from a wCTE in the ON CONFLICT's SET.
1191+
*/
1192+
11841193
/* Execute UPDATE with projection */
1185-
*returning=ExecUpdate(&tuple.t_data->t_ctid,NULL,
1194+
*returning=ExecUpdate(&tuple.t_self,NULL,
11861195
mtstate->mt_conflproj,planSlot,
11871196
&mtstate->mt_epqstate,mtstate->ps.state,
11881197
canSetTag);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,7 +1703,7 @@ create function upsert_after_func()
17031703
$$
17041704
begin
17051705
if (TG_OP = 'UPDATE') then
1706-
raise warning 'after update (old): %',new.*::text;
1706+
raise warning 'after update (old): %',old.*::text;
17071707
raise warning 'after update (new): %', new.*::text;
17081708
elsif (TG_OP = 'INSERT') then
17091709
raise warning 'after insert (new): %', new.*::text;
@@ -1724,7 +1724,7 @@ insert into upsert values(3, 'orange') on conflict (key) do update set color = '
17241724
WARNING: before insert (new): (3,orange)
17251725
WARNING: before update (old): (3,"red trig modified")
17261726
WARNING: before update (new): (3,"updated red trig modified")
1727-
WARNING: after update (old): (3,"updatedred trig modified")
1727+
WARNING: after update (old): (3,"red trig modified")
17281728
WARNING: after update (new): (3,"updated red trig modified")
17291729
insert into upsert values(4, 'green') on conflict (key) do update set color = 'updated ' || upsert.color;
17301730
WARNING: before insert (new): (4,green)
@@ -1734,7 +1734,7 @@ insert into upsert values(5, 'purple') on conflict (key) do update set color = '
17341734
WARNING: before insert (new): (5,purple)
17351735
WARNING: before update (old): (5,"green trig modified")
17361736
WARNING: before update (new): (5,"updated green trig modified")
1737-
WARNING: after update (old): (5,"updatedgreen trig modified")
1737+
WARNING: after update (old): (5,"green trig modified")
17381738
WARNING: after update (new): (5,"updated green trig modified")
17391739
insert into upsert values(6, 'white') on conflict (key) do update set color = 'updated ' || upsert.color;
17401740
WARNING: before insert (new): (6,white)
@@ -1744,7 +1744,7 @@ insert into upsert values(7, 'pink') on conflict (key) do update set color = 'up
17441744
WARNING: before insert (new): (7,pink)
17451745
WARNING: before update (old): (7,"white trig modified")
17461746
WARNING: before update (new): (7,"updated white trig modified")
1747-
WARNING: after update (old): (7,"updatedwhite trig modified")
1747+
WARNING: after update (old): (7,"white trig modified")
17481748
WARNING: after update (new): (7,"updated white trig modified")
17491749
insert into upsert values(8, 'yellow') on conflict (key) do update set color = 'updated ' || upsert.color;
17501750
WARNING: before insert (new): (8,yellow)

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1875,8 +1875,9 @@ ON CONFLICT (k) DO UPDATE SET v = (SELECT b || ' update' FROM aa WHERE a = 'a' L
18751875
WITH aa AS (SELECT 1 a, 2 b)
18761876
INSERT INTO z VALUES(1, (SELECT b || ' insert' FROM aa WHERE a = 1 ))
18771877
ON CONFLICT (k) DO UPDATE SET v = (SELECT b || ' update' FROM aa WHERE a = 1 LIMIT 1);
1878-
-- This shows an attempt to update an invisible row, which should really be
1879-
-- reported as a cardinality violation, but it doesn't seem worth fixing:
1878+
-- Update a row more than once, in different parts of a wCTE. That is
1879+
-- an allowed, presumably very rare, edge case, but since it was
1880+
-- broken in the past, having a test seems worthwhile.
18801881
WITH simpletup AS (
18811882
SELECT 2 k, 'Green' v),
18821883
upsert_cte AS (
@@ -1886,7 +1887,10 @@ upsert_cte AS (
18861887
INSERT INTO z VALUES(2, 'Red') ON CONFLICT (k) DO
18871888
UPDATE SET (k, v) = (SELECT k, v FROM upsert_cte WHERE upsert_cte.k = z.k)
18881889
RETURNING k, v;
1889-
ERROR: attempted to update invisible tuple
1890+
k | v
1891+
---+---
1892+
(0 rows)
1893+
18901894
DROP TABLE z;
18911895
-- check that run to completion happens in proper ordering
18921896
TRUNCATE TABLE y;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,7 @@ create function upsert_after_func()
12151215
$$
12161216
begin
12171217
if (TG_OP='UPDATE') then
1218-
raise warning'after update (old): %',new.*::text;
1218+
raise warning'after update (old): %',old.*::text;
12191219
raise warning'after update (new): %', new.*::text;
12201220
elsif (TG_OP='INSERT') then
12211221
raise warning'after insert (new): %', new.*::text;

‎src/test/regress/sql/with.sql

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -838,8 +838,9 @@ WITH aa AS (SELECT 1 a, 2 b)
838838
INSERT INTO zVALUES(1, (SELECT b||' insert'FROM aaWHERE a=1 ))
839839
ON CONFLICT (k) DOUPDATESET v= (SELECT b||' update'FROM aaWHERE a=1LIMIT1);
840840

841-
-- This shows an attempt to update an invisible row, which should really be
842-
-- reported as a cardinality violation, but it doesn't seem worth fixing:
841+
-- Update a row more than once, in different parts of a wCTE. That is
842+
-- an allowed, presumably very rare, edge case, but since it was
843+
-- broken in the past, having a test seems worthwhile.
843844
WITH simpletupAS (
844845
SELECT2 k,'Green' v),
845846
upsert_cteAS (

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp