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

Commit34263e8

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 parent5b51805 commit34263e8

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
@@ -1771,7 +1771,7 @@ create function upsert_after_func()
17711771
$$
17721772
begin
17731773
if (TG_OP = 'UPDATE') then
1774-
raise warning 'after update (old): %',new.*::text;
1774+
raise warning 'after update (old): %',old.*::text;
17751775
raise warning 'after update (new): %', new.*::text;
17761776
elsif (TG_OP = 'INSERT') then
17771777
raise warning 'after insert (new): %', new.*::text;
@@ -1792,7 +1792,7 @@ insert into upsert values(3, 'orange') on conflict (key) do update set color = '
17921792
WARNING: before insert (new): (3,orange)
17931793
WARNING: before update (old): (3,"red trig modified")
17941794
WARNING: before update (new): (3,"updated red trig modified")
1795-
WARNING: after update (old): (3,"updatedred trig modified")
1795+
WARNING: after update (old): (3,"red trig modified")
17961796
WARNING: after update (new): (3,"updated red trig modified")
17971797
insert into upsert values(4, 'green') on conflict (key) do update set color = 'updated ' || upsert.color;
17981798
WARNING: before insert (new): (4,green)
@@ -1802,7 +1802,7 @@ insert into upsert values(5, 'purple') on conflict (key) do update set color = '
18021802
WARNING: before insert (new): (5,purple)
18031803
WARNING: before update (old): (5,"green trig modified")
18041804
WARNING: before update (new): (5,"updated green trig modified")
1805-
WARNING: after update (old): (5,"updatedgreen trig modified")
1805+
WARNING: after update (old): (5,"green trig modified")
18061806
WARNING: after update (new): (5,"updated green trig modified")
18071807
insert into upsert values(6, 'white') on conflict (key) do update set color = 'updated ' || upsert.color;
18081808
WARNING: before insert (new): (6,white)
@@ -1812,7 +1812,7 @@ insert into upsert values(7, 'pink') on conflict (key) do update set color = 'up
18121812
WARNING: before insert (new): (7,pink)
18131813
WARNING: before update (old): (7,"white trig modified")
18141814
WARNING: before update (new): (7,"updated white trig modified")
1815-
WARNING: after update (old): (7,"updatedwhite trig modified")
1815+
WARNING: after update (old): (7,"white trig modified")
18161816
WARNING: after update (new): (7,"updated white trig modified")
18171817
insert into upsert values(8, 'yellow') on conflict (key) do update set color = 'updated ' || upsert.color;
18181818
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