Skip to content

Commit 55d85f4

Browse files
tglsfdctglsfdc
committed
Repair RI trigger visibility problems (this time for sure ;-)) per recent
discussion on pgsql-hackers: in READ COMMITTED mode we just have to force a QuerySnapshot update in the trigger, but in SERIALIZABLE mode we have to run the scan under a current snapshot and then complain if any rows would be updated/deleted that are not visible in the transaction snapshot.
1 parent 6099bc0 commit 55d85f4

File tree

15 files changed

+210
-84
lines changed

15 files changed

+210
-84
lines changed

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

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.156 2003/09/25 06:57:56 petere Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.157 2003/10/01 21:30:52 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1207,14 +1207,23 @@ simple_heap_insert(Relation relation, HeapTuple tup)
12071207
* NB: do not call this directly unless you are prepared to deal with
12081208
* concurrent-update conditions. Use simple_heap_delete instead.
12091209
*
1210+
* relation - table to be modified
1211+
* tid - TID of tuple to be deleted
1212+
* ctid - output parameter, used only for failure case (see below)
1213+
* cid - delete command ID to use in verifying tuple visibility
1214+
* crosscheck - if not SnapshotAny, also check tuple against this
1215+
* wait - true if should wait for any conflicting update to commit/abort
1216+
*
12101217
* Normal, successful return value is HeapTupleMayBeUpdated, which
12111218
* actually means we did delete it. Failure return codes are
12121219
* HeapTupleSelfUpdated, HeapTupleUpdated, or HeapTupleBeingUpdated
1213-
* (the last only possible if wait == false).
1220+
* (the last only possible if wait == false). On a failure return,
1221+
* *ctid is set to the ctid link of the target tuple (possibly a later
1222+
* version of the row).
12141223
*/
12151224
int
12161225
heap_delete(Relation relation, ItemPointer tid,
1217-
ItemPointer ctid, CommandId cid, bool wait)
1226+
ItemPointer ctid, CommandId cid, Snapshot crosscheck, bool wait)
12181227
{
12191228
ItemId lp;
12201229
HeapTupleData tp;
@@ -1240,7 +1249,7 @@ heap_delete(Relation relation, ItemPointer tid,
12401249
tp.t_tableOid = relation->rd_id;
12411250

12421251
l1:
1243-
result = HeapTupleSatisfiesUpdate(&tp, cid);
1252+
result = HeapTupleSatisfiesUpdate(tp.t_data, cid);
12441253

12451254
if (result == HeapTupleInvisible)
12461255
{
@@ -1278,6 +1287,14 @@ heap_delete(Relation relation, ItemPointer tid,
12781287
else
12791288
result = HeapTupleUpdated;
12801289
}
1290+
1291+
if (crosscheck != SnapshotAny && result == HeapTupleMayBeUpdated)
1292+
{
1293+
/* Perform additional check for serializable RI updates */
1294+
if (!HeapTupleSatisfiesSnapshot(tp.t_data, crosscheck))
1295+
result = HeapTupleUpdated;
1296+
}
1297+
12811298
if (result != HeapTupleMayBeUpdated)
12821299
{
12831300
Assert(result == HeapTupleSelfUpdated ||
@@ -1378,7 +1395,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
13781395

13791396
result = heap_delete(relation, tid,
13801397
&ctid,
1381-
GetCurrentCommandId(),
1398+
GetCurrentCommandId(), SnapshotAny,
13821399
true /* wait for commit */);
13831400
switch (result)
13841401
{
@@ -1407,14 +1424,26 @@ simple_heap_delete(Relation relation, ItemPointer tid)
14071424
* NB: do not call this directly unless you are prepared to deal with
14081425
* concurrent-update conditions. Use simple_heap_update instead.
14091426
*
1427+
* relation - table to be modified
1428+
* otid - TID of old tuple to be replaced
1429+
* newtup - newly constructed tuple data to store
1430+
* ctid - output parameter, used only for failure case (see below)
1431+
* cid - update command ID to use in verifying old tuple visibility
1432+
* crosscheck - if not SnapshotAny, also check old tuple against this
1433+
* wait - true if should wait for any conflicting update to commit/abort
1434+
*
14101435
* Normal, successful return value is HeapTupleMayBeUpdated, which
14111436
* actually means we *did* update it. Failure return codes are
14121437
* HeapTupleSelfUpdated, HeapTupleUpdated, or HeapTupleBeingUpdated
1413-
* (the last only possible if wait == false).
1438+
* (the last only possible if wait == false). On a failure return,
1439+
* *ctid is set to the ctid link of the old tuple (possibly a later
1440+
* version of the row).
1441+
* On success, newtup->t_self is set to the TID where the new tuple
1442+
* was inserted.
14141443
*/
14151444
int
14161445
heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
1417-
ItemPointer ctid, CommandId cid, bool wait)
1446+
ItemPointer ctid, CommandId cid, Snapshot crosscheck, bool wait)
14181447
{
14191448
ItemId lp;
14201449
HeapTupleData oldtup;
@@ -1450,7 +1479,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
14501479
*/
14511480

14521481
l2:
1453-
result = HeapTupleSatisfiesUpdate(&oldtup, cid);
1482+
result = HeapTupleSatisfiesUpdate(oldtup.t_data, cid);
14541483

14551484
if (result == HeapTupleInvisible)
14561485
{
@@ -1488,6 +1517,14 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
14881517
else
14891518
result = HeapTupleUpdated;
14901519
}
1520+
1521+
if (crosscheck != SnapshotAny && result == HeapTupleMayBeUpdated)
1522+
{
1523+
/* Perform additional check for serializable RI updates */
1524+
if (!HeapTupleSatisfiesSnapshot(oldtup.t_data, crosscheck))
1525+
result = HeapTupleUpdated;
1526+
}
1527+
14911528
if (result != HeapTupleMayBeUpdated)
14921529
{
14931530
Assert(result == HeapTupleSelfUpdated ||
@@ -1718,7 +1755,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
17181755

17191756
result = heap_update(relation, otid, tup,
17201757
&ctid,
1721-
GetCurrentCommandId(),
1758+
GetCurrentCommandId(), SnapshotAny,
17221759
true /* wait for commit */);
17231760
switch (result)
17241761
{
@@ -1767,7 +1804,7 @@ heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer,
17671804
tuple->t_len = ItemIdGetLength(lp);
17681805

17691806
l3:
1770-
result = HeapTupleSatisfiesUpdate(tuple, cid);
1807+
result = HeapTupleSatisfiesUpdate(tuple->t_data, cid);
17711808

17721809
if (result == HeapTupleInvisible)
17731810
{

‎src/backend/commands/async.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/commands/async.c,v 1.100 2003/09/15 23:33:39 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/commands/async.c,v 1.101 2003/10/01 21:30:52 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -537,7 +537,7 @@ AtCommit_Notify(void)
537537
*/
538538
result = heap_update(lRel, &lTuple->t_self, rTuple,
539539
&ctid,
540-
GetCurrentCommandId(),
540+
GetCurrentCommandId(), SnapshotAny,
541541
false /* no wait for commit */);
542542
switch (result)
543543
{

‎src/backend/executor/execMain.c‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.219 2003/09/25 18:58:35 tgl Exp $
29+
* $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.220 2003/10/01 21:30:52 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -104,8 +104,14 @@ static void EvalPlanQualStop(evalPlanQual *epq);
104104
* field of the QueryDesc is filled in to describe the tuples that will be
105105
* returned, and the internal fields (estate and planstate) are set up.
106106
*
107-
* If useSnapshotNow is true, run the query with SnapshotNow time qual rules
108-
* instead of the normal use of QuerySnapshot.
107+
* If useCurrentSnapshot is true, run the query with the latest available
108+
* snapshot, instead of the normal QuerySnapshot. Also, if it's an update
109+
* or delete query, check that the rows to be updated or deleted would be
110+
* visible to the normal QuerySnapshot. (This is a special-case behavior
111+
* needed for referential integrity updates in serializable transactions.
112+
* We must check all currently-committed rows, but we want to throw a
113+
* can't-serialize error if any rows that would need updates would not be
114+
* visible under the normal serializable snapshot.)
109115
*
110116
* If explainOnly is true, we are not actually intending to run the plan,
111117
* only to set up for EXPLAIN; so skip unwanted side-effects.
@@ -115,7 +121,7 @@ static void EvalPlanQualStop(evalPlanQual *epq);
115121
* ----------------------------------------------------------------
116122
*/
117123
void
118-
ExecutorStart(QueryDesc *queryDesc, bool useSnapshotNow, bool explainOnly)
124+
ExecutorStart(QueryDesc *queryDesc, bool useCurrentSnapshot, bool explainOnly)
119125
{
120126
EState *estate;
121127
MemoryContext oldcontext;
@@ -157,15 +163,18 @@ ExecutorStart(QueryDesc *queryDesc, bool useSnapshotNow, bool explainOnly)
157163
* the life of this query, even if it outlives the current command and
158164
* current snapshot.
159165
*/
160-
if (useSnapshotNow)
166+
if (useCurrentSnapshot)
161167
{
162-
estate->es_snapshot = SnapshotNow;
163-
estate->es_snapshot_cid = GetCurrentCommandId();
168+
/* RI update/delete query --- must use an up-to-date snapshot */
169+
estate->es_snapshot = CopyCurrentSnapshot();
170+
/* crosscheck updates/deletes against transaction snapshot */
171+
estate->es_crosscheck_snapshot = CopyQuerySnapshot();
164172
}
165173
else
166174
{
175+
/* normal query --- use query snapshot, no crosscheck */
167176
estate->es_snapshot = CopyQuerySnapshot();
168-
estate->es_snapshot_cid = estate->es_snapshot->curcid;
177+
estate->es_crosscheck_snapshot = SnapshotAny;
169178
}
170179

171180
/*
@@ -1118,7 +1127,7 @@ lnext: ;
11181127

11191128
tuple.t_self = *((ItemPointer) DatumGetPointer(datum));
11201129
test = heap_mark4update(erm->relation, &tuple, &buffer,
1121-
estate->es_snapshot_cid);
1130+
estate->es_snapshot->curcid);
11221131
ReleaseBuffer(buffer);
11231132
switch (test)
11241133
{
@@ -1278,7 +1287,7 @@ ExecSelect(TupleTableSlot *slot,
12781287
if (estate->es_into_relation_descriptor != NULL)
12791288
{
12801289
heap_insert(estate->es_into_relation_descriptor, tuple,
1281-
estate->es_snapshot_cid);
1290+
estate->es_snapshot->curcid);
12821291
IncrAppended();
12831292
}
12841293

@@ -1354,7 +1363,7 @@ ExecInsert(TupleTableSlot *slot,
13541363
* insert the tuple
13551364
*/
13561365
newId = heap_insert(resultRelationDesc, tuple,
1357-
estate->es_snapshot_cid);
1366+
estate->es_snapshot->curcid);
13581367

13591368
IncrAppended();
13601369
(estate->es_processed)++;
@@ -1406,7 +1415,7 @@ ExecDelete(TupleTableSlot *slot,
14061415
bool dodelete;
14071416

14081417
dodelete = ExecBRDeleteTriggers(estate, resultRelInfo, tupleid,
1409-
estate->es_snapshot_cid);
1418+
estate->es_snapshot->curcid);
14101419

14111420
if (!dodelete) /* "do nothing" */
14121421
return;
@@ -1418,7 +1427,8 @@ ExecDelete(TupleTableSlot *slot,
14181427
ldelete:;
14191428
result = heap_delete(resultRelationDesc, tupleid,
14201429
&ctid,
1421-
estate->es_snapshot_cid,
1430+
estate->es_snapshot->curcid,
1431+
estate->es_crosscheck_snapshot,
14221432
true /* wait for commit */);
14231433
switch (result)
14241434
{
@@ -1517,7 +1527,7 @@ ExecUpdate(TupleTableSlot *slot,
15171527

15181528
newtuple = ExecBRUpdateTriggers(estate, resultRelInfo,
15191529
tupleid, tuple,
1520-
estate->es_snapshot_cid);
1530+
estate->es_snapshot->curcid);
15211531

15221532
if (newtuple == NULL) /* "do nothing" */
15231533
return;
@@ -1553,7 +1563,8 @@ lreplace:;
15531563
*/
15541564
result = heap_update(resultRelationDesc, tupleid, tuple,
15551565
&ctid,
1556-
estate->es_snapshot_cid,
1566+
estate->es_snapshot->curcid,
1567+
estate->es_crosscheck_snapshot,
15571568
true /* wait for commit */);
15581569
switch (result)
15591570
{
@@ -2039,7 +2050,7 @@ EvalPlanQualStart(evalPlanQual *epq, EState *estate, evalPlanQual *priorepq)
20392050
*/
20402051
epqstate->es_direction = ForwardScanDirection;
20412052
epqstate->es_snapshot = estate->es_snapshot;
2042-
epqstate->es_snapshot_cid = estate->es_snapshot_cid;
2053+
epqstate->es_crosscheck_snapshot = estate->es_crosscheck_snapshot;
20432054
epqstate->es_range_table = estate->es_range_table;
20442055
epqstate->es_result_relations = estate->es_result_relations;
20452056
epqstate->es_num_result_relations = estate->es_num_result_relations;

‎src/backend/executor/execUtils.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.105 2003/09/25 18:58:35 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.106 2003/10/01 21:30:52 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -178,7 +178,7 @@ CreateExecutorState(void)
178178
*/
179179
estate->es_direction = ForwardScanDirection;
180180
estate->es_snapshot = SnapshotNow;
181-
estate->es_snapshot_cid = FirstCommandId;
181+
estate->es_crosscheck_snapshot = SnapshotAny; /* means no crosscheck */
182182
estate->es_range_table = NIL;
183183

184184
estate->es_result_relations = NULL;

‎src/backend/executor/nodeSubplan.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v 1.57 2003/09/25 18:58:35 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v 1.58 2003/10/01 21:30:52 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -709,7 +709,7 @@ ExecInitSubPlan(SubPlanState *node, EState *estate)
709709
sp_estate->es_tupleTable =
710710
ExecCreateTupleTable(ExecCountSlotsNode(subplan->plan) + 10);
711711
sp_estate->es_snapshot = estate->es_snapshot;
712-
sp_estate->es_snapshot_cid = estate->es_snapshot_cid;
712+
sp_estate->es_crosscheck_snapshot = estate->es_crosscheck_snapshot;
713713
sp_estate->es_instrument = estate->es_instrument;
714714

715715
/*

‎src/backend/executor/nodeSubqueryscan.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
*
1313
*
1414
* IDENTIFICATION
15-
* $Header: /cvsroot/pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.21 2003/09/25 18:58:35 tgl Exp $
15+
* $Header: /cvsroot/pgsql/src/backend/executor/nodeSubqueryscan.c,v 1.22 2003/10/01 21:30:52 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -177,7 +177,7 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate)
177177
sp_estate->es_tupleTable =
178178
ExecCreateTupleTable(ExecCountSlotsNode(node->subplan) + 10);
179179
sp_estate->es_snapshot = estate->es_snapshot;
180-
sp_estate->es_snapshot_cid = estate->es_snapshot_cid;
180+
sp_estate->es_crosscheck_snapshot = estate->es_crosscheck_snapshot;
181181
sp_estate->es_instrument = estate->es_instrument;
182182

183183
/*

0 commit comments

Comments
 (0)