Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 786f1a5

Browse files
tglsfdctglsfdc
committed
Fix all the places that called heap_update() and heap_delete() without
bothering to check the return value --- which meant that in case the update or delete failed because of a concurrent update, you'd not find out about it, except by observing later that the transaction produced the wrong outcome. There are now subroutines simple_heap_update and simple_heap_delete that should be used anyplace that you're not prepared to do the full nine yards of coping with concurrent updates. In practice, that seems to mean absolutely everywhere but the executor, because *noplace* else was checking.
1 parent 7a2a1ac commit 786f1a5

File tree

24 files changed

+192
-107
lines changed

24 files changed

+192
-107
lines changed

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

Lines changed: 82 additions & 5 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.108 2001/01/15 05:29:19 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.109 2001/01/23 04:32:20 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1423,6 +1423,9 @@ heap_insert(Relation relation, HeapTuple tup)
14231423

14241424
/*
14251425
* heap_delete - delete a tuple
1426+
*
1427+
* NB: do not call this directly unless you are prepared to deal with
1428+
* concurrent-update conditions. Use simple_heap_delete instead.
14261429
*/
14271430
int
14281431
heap_delete(Relation relation, ItemPointer tid, ItemPointer ctid)
@@ -1496,8 +1499,7 @@ heap_delete(Relation relation, ItemPointer tid, ItemPointer ctid)
14961499
if (result != HeapTupleMayBeUpdated)
14971500
{
14981501
Assert(result == HeapTupleSelfUpdated || result == HeapTupleUpdated);
1499-
if (ctid != NULL)
1500-
*ctid = tp.t_data->t_ctid;
1502+
*ctid = tp.t_data->t_ctid;
15011503
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
15021504
ReleaseBuffer(buffer);
15031505
return result;
@@ -1560,8 +1562,48 @@ heap_delete(Relation relation, ItemPointer tid, ItemPointer ctid)
15601562
return HeapTupleMayBeUpdated;
15611563
}
15621564

1565+
/*
1566+
* simple_heap_delete - delete a tuple
1567+
*
1568+
* This routine may be used to delete a tuple when concurrent updates of
1569+
* the target tuple are not expected (for example, because we have a lock
1570+
* on the relation associated with the tuple). Any failure is reported
1571+
* via elog().
1572+
*/
1573+
void
1574+
simple_heap_delete(Relation relation, ItemPointer tid)
1575+
{
1576+
ItemPointerData ctid;
1577+
int result;
1578+
1579+
result = heap_delete(relation, tid, &ctid);
1580+
switch (result)
1581+
{
1582+
case HeapTupleSelfUpdated:
1583+
/* Tuple was already updated in current command? */
1584+
elog(ERROR, "simple_heap_delete: tuple already updated by self");
1585+
break;
1586+
1587+
case HeapTupleMayBeUpdated:
1588+
/* done successfully */
1589+
break;
1590+
1591+
case HeapTupleUpdated:
1592+
elog(ERROR, "simple_heap_delete: tuple concurrently updated");
1593+
break;
1594+
1595+
default:
1596+
elog(ERROR, "Unknown status %u from heap_delete", result);
1597+
break;
1598+
}
1599+
1600+
}
1601+
15631602
/*
15641603
* heap_update - replace a tuple
1604+
*
1605+
* NB: do not call this directly unless you are prepared to deal with
1606+
* concurrent-update conditions. Use simple_heap_update instead.
15651607
*/
15661608
int
15671609
heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
@@ -1643,8 +1685,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
16431685
if (result != HeapTupleMayBeUpdated)
16441686
{
16451687
Assert(result == HeapTupleSelfUpdated || result == HeapTupleUpdated);
1646-
if (ctid != NULL)
1647-
*ctid = oldtup.t_data->t_ctid;
1688+
*ctid = oldtup.t_data->t_ctid;
16481689
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
16491690
ReleaseBuffer(buffer);
16501691
return result;
@@ -1783,6 +1824,42 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
17831824
return HeapTupleMayBeUpdated;
17841825
}
17851826

1827+
/*
1828+
* simple_heap_update - replace a tuple
1829+
*
1830+
* This routine may be used to update a tuple when concurrent updates of
1831+
* the target tuple are not expected (for example, because we have a lock
1832+
* on the relation associated with the tuple). Any failure is reported
1833+
* via elog().
1834+
*/
1835+
void
1836+
simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
1837+
{
1838+
ItemPointerData ctid;
1839+
int result;
1840+
1841+
result = heap_update(relation, otid, tup, &ctid);
1842+
switch (result)
1843+
{
1844+
case HeapTupleSelfUpdated:
1845+
/* Tuple was already updated in current command? */
1846+
elog(ERROR, "simple_heap_update: tuple already updated by self");
1847+
break;
1848+
1849+
case HeapTupleMayBeUpdated:
1850+
/* done successfully */
1851+
break;
1852+
1853+
case HeapTupleUpdated:
1854+
elog(ERROR, "simple_heap_update: tuple concurrently updated");
1855+
break;
1856+
1857+
default:
1858+
elog(ERROR, "Unknown status %u from heap_update", result);
1859+
break;
1860+
}
1861+
}
1862+
17861863
/*
17871864
* heap_mark4update - mark a tuple for update
17881865
*/

‎src/backend/access/heap/tuptoaster.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/access/heap/tuptoaster.c,v 1.14 2001/01/15 05:29:19 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.15 2001/01/23 04:32:20 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -937,7 +937,7 @@ toast_delete_datum(Relation rel, Datum value)
937937
* Have a chunk, delete it
938938
* ----------
939939
*/
940-
heap_delete(toastrel, &toasttup.t_self, NULL);
940+
simple_heap_delete(toastrel, &toasttup.t_self);
941941

942942
ReleaseBuffer(buffer);
943943
}

‎src/backend/catalog/aclchk.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/catalog/aclchk.c,v 1.44 2000/11/28 23:42:31 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.45 2001/01/23 04:32:21 tgl Exp $
1212
*
1313
* NOTES
1414
* See acl.h.
@@ -139,7 +139,7 @@ ChangeAcl(char *relname,
139139

140140
ReleaseSysCache(tuple);
141141

142-
heap_update(relation, &newtuple->t_self, newtuple, NULL);
142+
simple_heap_update(relation, &newtuple->t_self, newtuple);
143143

144144
/* keep the catalog indices up to date */
145145
CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices,

‎src/backend/catalog/heap.c

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.156 2001/01/01 21:33:31 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.157 2001/01/23 04:32:21 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1000,7 +1000,7 @@ RelationRemoveInheritance(Relation relation)
10001000

10011001
while (HeapTupleIsValid(tuple = heap_getnext(scan, 0)))
10021002
{
1003-
heap_delete(catalogRelation, &tuple->t_self, NULL);
1003+
simple_heap_delete(catalogRelation, &tuple->t_self);
10041004
found = true;
10051005
}
10061006

@@ -1023,7 +1023,9 @@ RelationRemoveInheritance(Relation relation)
10231023
&entry);
10241024

10251025
while (HeapTupleIsValid(tuple = heap_getnext(scan, 0)))
1026-
heap_delete(catalogRelation, &tuple->t_self, NULL);
1026+
{
1027+
simple_heap_delete(catalogRelation, &tuple->t_self);
1028+
}
10271029

10281030
heap_endscan(scan);
10291031
heap_close(catalogRelation, RowExclusiveLock);
@@ -1093,7 +1095,7 @@ DeleteRelationTuple(Relation rel)
10931095
* delete the relation tuple from pg_class, and finish up.
10941096
* ----------------
10951097
*/
1096-
heap_delete(pg_class_desc, &tup->t_self, NULL);
1098+
simple_heap_delete(pg_class_desc, &tup->t_self);
10971099
heap_freetuple(tup);
10981100

10991101
heap_close(pg_class_desc, RowExclusiveLock);
@@ -1267,7 +1269,7 @@ DeleteAttributeTuples(Relation rel)
12671269
/*** Delete any comments associated with this attribute ***/
12681270
DeleteComments(tup->t_data->t_oid);
12691271

1270-
heap_delete(pg_attribute_desc, &tup->t_self, NULL);
1272+
simple_heap_delete(pg_attribute_desc, &tup->t_self);
12711273
heap_freetuple(tup);
12721274
}
12731275
}
@@ -1382,12 +1384,10 @@ DeleteTypeTuple(Relation rel)
13821384

13831385
/* ----------------
13841386
* Ok, it's safe so we delete the relation tuple
1385-
* from pg_type and finish up. But first end the scan so that
1386-
* we release the read lock on pg_type. -mer 13 Aug 1991
1387+
* from pg_type and finish up.
13871388
* ----------------
13881389
*/
1389-
1390-
heap_delete(pg_type_desc, &tup->t_self, NULL);
1390+
simple_heap_delete(pg_type_desc, &tup->t_self);
13911391

13921392
heap_endscan(pg_type_scan);
13931393
heap_close(pg_type_desc, RowExclusiveLock);
@@ -1595,7 +1595,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, char *adbin,
15951595
if (!attStruct->atthasdef)
15961596
{
15971597
attStruct->atthasdef = true;
1598-
heap_update(attrrel, &atttup->t_self, atttup, NULL);
1598+
simple_heap_update(attrrel, &atttup->t_self, atttup);
15991599
/* keep catalog indices current */
16001600
CatalogOpenIndices(Num_pg_attr_indices, Name_pg_attr_indices,
16011601
attridescs);
@@ -1962,7 +1962,7 @@ AddRelationRawConstraints(Relation rel,
19621962

19631963
relStruct->relchecks = numchecks;
19641964

1965-
heap_update(relrel, &reltup->t_self, reltup, NULL);
1965+
simple_heap_update(relrel, &reltup->t_self, reltup);
19661966

19671967
/* keep catalog indices current */
19681968
CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices,
@@ -1990,7 +1990,9 @@ RemoveAttrDefault(Relation rel)
19901990
adscan = heap_beginscan(adrel, 0, SnapshotNow, 1, &key);
19911991

19921992
while (HeapTupleIsValid(tup = heap_getnext(adscan, 0)))
1993-
heap_delete(adrel, &tup->t_self, NULL);
1993+
{
1994+
simple_heap_delete(adrel, &tup->t_self);
1995+
}
19941996

19951997
heap_endscan(adscan);
19961998
heap_close(adrel, RowExclusiveLock);
@@ -2012,7 +2014,9 @@ RemoveRelCheck(Relation rel)
20122014
rcscan = heap_beginscan(rcrel, 0, SnapshotNow, 1, &key);
20132015

20142016
while (HeapTupleIsValid(tup = heap_getnext(rcscan, 0)))
2015-
heap_delete(rcrel, &tup->t_self, NULL);
2017+
{
2018+
simple_heap_delete(rcrel, &tup->t_self);
2019+
}
20162020

20172021
heap_endscan(rcscan);
20182022
heap_close(rcrel, RowExclusiveLock);
@@ -2049,7 +2053,9 @@ RemoveStatistics(Relation rel)
20492053
scan = heap_beginscan(pgstatistic, false, SnapshotNow, 1, &key);
20502054

20512055
while (HeapTupleIsValid(tuple = heap_getnext(scan, 0)))
2052-
heap_delete(pgstatistic, &tuple->t_self, NULL);
2056+
{
2057+
simple_heap_delete(pgstatistic, &tuple->t_self);
2058+
}
20532059

20542060
heap_endscan(scan);
20552061
heap_close(pgstatistic, RowExclusiveLock);

‎src/backend/catalog/index.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.135 2001/01/18 07:29:04 inoue Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.136 2001/01/23 04:32:21 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -783,7 +783,7 @@ UpdateIndexPredicate(Oid indexoid, Node *oldPred, Node *predicate)
783783

784784
newtup = heap_modifytuple(tuple, pg_index, values, nulls, replace);
785785

786-
heap_update(pg_index, &newtup->t_self, newtup, NULL);
786+
simple_heap_update(pg_index, &newtup->t_self, newtup);
787787

788788
heap_freetuple(newtup);
789789
ReleaseSysCache(tuple);
@@ -1085,7 +1085,7 @@ index_drop(Oid indexId)
10851085
elog(ERROR, "index_drop: cache lookup failed for index %u",
10861086
indexId);
10871087

1088-
heap_delete(relationRelation, &tuple->t_self, NULL);
1088+
simple_heap_delete(relationRelation, &tuple->t_self);
10891089
heap_freetuple(tuple);
10901090

10911091
/*
@@ -1113,7 +1113,7 @@ index_drop(Oid indexId)
11131113
Int16GetDatum(attnum),
11141114
0, 0)))
11151115
{
1116-
heap_delete(attributeRelation, &tuple->t_self, NULL);
1116+
simple_heap_delete(attributeRelation, &tuple->t_self);
11171117
heap_freetuple(tuple);
11181118
attnum++;
11191119
}
@@ -1132,7 +1132,7 @@ index_drop(Oid indexId)
11321132
elog(ERROR, "index_drop: cache lookup failed for index %u",
11331133
indexId);
11341134

1135-
heap_delete(indexRelation, &tuple->t_self, NULL);
1135+
simple_heap_delete(indexRelation, &tuple->t_self);
11361136
heap_freetuple(tuple);
11371137
heap_close(indexRelation, RowExclusiveLock);
11381138

@@ -1495,7 +1495,7 @@ setRelhasindex(Oid relid, bool hasindex)
14951495
}
14961496
else
14971497
{
1498-
heap_update(pg_class, &tuple->t_self, tuple, NULL);
1498+
simple_heap_update(pg_class, &tuple->t_self, tuple);
14991499

15001500
/* Keep the catalog indices up to date */
15011501
if (!IsIgnoringSystemIndexes())
@@ -1545,7 +1545,7 @@ setNewRelfilenode(Relation relation)
15451545
classTuple = heap_copytuple(&lockTupleData);
15461546
ReleaseBuffer(buffer);
15471547
((Form_pg_class) GETSTRUCT(classTuple))->relfilenode = newrelfilenode;
1548-
heap_update(pg_class, &classTuple->t_self, classTuple, NULL);
1548+
simple_heap_update(pg_class, &classTuple->t_self, classTuple);
15491549
}
15501550
/* unlink old relfilenode */
15511551
DropRelationBuffers(relation);
@@ -1751,7 +1751,7 @@ UpdateStats(Oid relid, long reltuples)
17511751
replace[Anum_pg_class_reltuples - 1] = 'r';
17521752
values[Anum_pg_class_reltuples - 1] = (Datum) reltuples;
17531753
newtup = heap_modifytuple(tuple, pg_class, values, nulls, replace);
1754-
heap_update(pg_class, &tuple->t_self, newtup, NULL);
1754+
simple_heap_update(pg_class, &tuple->t_self, newtup);
17551755
if (!IsIgnoringSystemIndexes())
17561756
{
17571757
CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices, idescs);

‎src/backend/catalog/pg_largeobject.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/catalog/pg_largeobject.c,v 1.5 2000/10/24 01:38:23 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/pg_largeobject.c,v 1.6 2001/01/23 04:32:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -118,7 +118,7 @@ LargeObjectDrop(Oid loid)
118118
pfree(indexRes);
119119
if (tuple.t_data != NULL)
120120
{
121-
heap_delete(pg_largeobject, &tuple.t_self, NULL);
121+
simple_heap_delete(pg_largeobject, &tuple.t_self);
122122
ReleaseBuffer(buffer);
123123
found = true;
124124
}

0 commit comments

Comments
 (0)