Skip to content

Commit 9fee323

Browse files
hlinnakahlinnaka
committed
Fix assertions with RI triggers in heap_update and heap_delete.
If the tuple being updated is not visible to the crosscheck snapshot, we return TM_Updated but the assertions would not hold in that case. Move them to before the cross-check. Fixes bug #17893. Backpatch to all supported versions. Author: Alexander Lakhin Backpatch-through: 12 Discussion: https://www.postgresql.org/message-id/17893-35847009eec517b5%40postgresql.org
1 parent 07cb7bc commit 9fee323

File tree

4 files changed

+64
-20
lines changed

4 files changed

+64
-20
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2717,13 +2717,7 @@ heap_delete(Relation relation, ItemPointer tid,
27172717
result = TM_Deleted;
27182718
}
27192719

2720-
if (crosscheck != InvalidSnapshot && result == TM_Ok)
2721-
{
2722-
/* Perform additional check for transaction-snapshot mode RI updates */
2723-
if (!HeapTupleSatisfiesVisibility(&tp, crosscheck, buffer))
2724-
result = TM_Updated;
2725-
}
2726-
2720+
/* sanity check the result HeapTupleSatisfiesUpdate() and the logic above */
27272721
if (result != TM_Ok)
27282722
{
27292723
Assert(result == TM_SelfModified ||
@@ -2733,6 +2727,17 @@ heap_delete(Relation relation, ItemPointer tid,
27332727
Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID));
27342728
Assert(result != TM_Updated ||
27352729
!ItemPointerEquals(&tp.t_self, &tp.t_data->t_ctid));
2730+
}
2731+
2732+
if (crosscheck != InvalidSnapshot && result == TM_Ok)
2733+
{
2734+
/* Perform additional check for transaction-snapshot mode RI updates */
2735+
if (!HeapTupleSatisfiesVisibility(&tp, crosscheck, buffer))
2736+
result = TM_Updated;
2737+
}
2738+
2739+
if (result != TM_Ok)
2740+
{
27362741
tmfd->ctid = tp.t_data->t_ctid;
27372742
tmfd->xmax = HeapTupleHeaderGetUpdateXid(tp.t_data);
27382743
if (result == TM_SelfModified)
@@ -3351,16 +3356,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
33513356
result = TM_Deleted;
33523357
}
33533358

3354-
if (crosscheck != InvalidSnapshot && result == TM_Ok)
3355-
{
3356-
/* Perform additional check for transaction-snapshot mode RI updates */
3357-
if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
3358-
{
3359-
result = TM_Updated;
3360-
Assert(!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
3361-
}
3362-
}
3363-
3359+
/* Sanity check the result HeapTupleSatisfiesUpdate() and the logic above */
33643360
if (result != TM_Ok)
33653361
{
33663362
Assert(result == TM_SelfModified ||
@@ -3370,6 +3366,17 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
33703366
Assert(!(oldtup.t_data->t_infomask & HEAP_XMAX_INVALID));
33713367
Assert(result != TM_Updated ||
33723368
!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
3369+
}
3370+
3371+
if (crosscheck != InvalidSnapshot && result == TM_Ok)
3372+
{
3373+
/* Perform additional check for transaction-snapshot mode RI updates */
3374+
if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
3375+
result = TM_Updated;
3376+
}
3377+
3378+
if (result != TM_Ok)
3379+
{
33733380
tmfd->ctid = oldtup.t_data->t_ctid;
33743381
tmfd->xmax = HeapTupleHeaderGetUpdateXid(oldtup.t_data);
33753382
if (result == TM_SelfModified)

‎src/include/access/tableam.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,8 +1479,8 @@ table_multi_insert(Relation rel, TupleTableSlot **slots, int nslots,
14791479
* TM_BeingModified (the last only possible if wait == false).
14801480
*
14811481
* In the failure cases, the routine fills *tmfd with the tuple's t_ctid,
1482-
* t_xmax, and, if possible, and, if possible, t_cmax. See comments for
1483-
* struct TM_FailureData for additional info.
1482+
* t_xmax, and, if possible, t_cmax. See comments for struct
1483+
* TM_FailureData for additional info.
14841484
*/
14851485
static inline TM_Result
14861486
table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid,

‎src/test/isolation/expected/fk-snapshot.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,25 @@ a
122122
1
123123
(1 row)
124124

125+
126+
starting permutation: s2ip2 s1brr s1ifp2 s2brr s2dp2 s1c s2c
127+
step s2ip2: INSERT INTO pk_noparted VALUES (2);
128+
step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ;
129+
step s1ifp2: INSERT INTO fk_parted_pk VALUES (2);
130+
step s2brr: BEGIN ISOLATION LEVEL REPEATABLE READ;
131+
step s2dp2: DELETE FROM pk_noparted WHERE a = 2; <waiting ...>
132+
step s1c: COMMIT;
133+
step s2dp2: <... completed>
134+
ERROR: could not serialize access due to concurrent update
135+
step s2c: COMMIT;
136+
137+
starting permutation: s2ip2 s1brr s1ifn2 s2brr s2dp2 s1c s2c
138+
step s2ip2: INSERT INTO pk_noparted VALUES (2);
139+
step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ;
140+
step s1ifn2: INSERT INTO fk_noparted_sn VALUES (2);
141+
step s2brr: BEGIN ISOLATION LEVEL REPEATABLE READ;
142+
step s2dp2: DELETE FROM pk_noparted WHERE a = 2; <waiting ...>
143+
step s1c: COMMIT;
144+
step s2dp2: <... completed>
145+
ERROR: could not serialize access due to concurrent update
146+
step s2c: COMMIT;

‎src/test/isolation/specs/fk-snapshot.spec

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,27 @@ setup
1313
CREATE TABLE fk_noparted (
1414
a int REFERENCES fk_parted_pk ON DELETE NO ACTION INITIALLY DEFERRED
1515
);
16+
17+
CREATE TABLE fk_noparted_sn (
18+
a int REFERENCES pk_noparted ON DELETE SET NULL
19+
);
20+
1621
INSERT INTO pk_noparted VALUES (1);
1722
INSERT INTO fk_parted_pk VALUES (1);
1823
INSERT INTO fk_noparted VALUES (1);
1924
}
2025

2126
teardown
2227
{
23-
DROP TABLE pk_noparted, fk_parted_pk, fk_noparted;
28+
DROP TABLE pk_noparted, fk_parted_pk, fk_noparted, fk_noparted_sn;
2429
}
2530

2631
session s1
2732
step s1brr { BEGIN ISOLATION LEVEL REPEATABLE READ; }
2833
step s1brc { BEGIN ISOLATION LEVEL READ COMMITTED; }
2934
step s1ifp2 { INSERT INTO fk_parted_pk VALUES (2); }
3035
step s1ifp1 { INSERT INTO fk_parted_pk VALUES (1); }
36+
step s1ifn2 { INSERT INTO fk_noparted_sn VALUES (2); }
3137
step s1dfp { DELETE FROM fk_parted_pk WHERE a = 1; }
3238
step s1c { COMMIT; }
3339
step s1sfp { SELECT * FROM fk_parted_pk; }
@@ -38,6 +44,7 @@ session s2
3844
step s2brr { BEGIN ISOLATION LEVEL REPEATABLE READ; }
3945
step s2brc { BEGIN ISOLATION LEVEL READ COMMITTED; }
4046
step s2ip2 { INSERT INTO pk_noparted VALUES (2); }
47+
step s2dp2 { DELETE FROM pk_noparted WHERE a = 2; }
4148
step s2ifn2 { INSERT INTO fk_noparted VALUES (2); }
4249
step s2c { COMMIT; }
4350
step s2sfp { SELECT * FROM fk_parted_pk; }
@@ -59,3 +66,11 @@ permutation s1brc s2brc s2ip2 s1sp s2c s1sp s1ifp2 s2brc s2sfp s1c s1sfp s2ifn2
5966
# the same no matter the snapshot mode
6067
permutation s1brr s1dfp s1ifp1 s1c s1sfn
6168
permutation s1brc s1dfp s1ifp1 s1c s1sfn
69+
70+
# trying to delete a row through DELETE CASCADE, whilst that row is deleted
71+
# in a concurrent transaction
72+
permutation s2ip2 s1brr s1ifp2 s2brr s2dp2 s1c s2c
73+
74+
# trying to update a row through DELETE SET NULL, whilst that row is deleted
75+
# in a concurrent transaction
76+
permutation s2ip2 s1brr s1ifn2 s2brr s2dp2 s1c s2c

0 commit comments

Comments
 (0)