Modify tuptoaster's API so that it does not try to modify the passed

tuple in-place, but instead passes back an all-new tuple structure if
any changes are needed.  This is a much cleaner and more robust solution
for the bug discovered by Alexey Beschiokov; accordingly, revert the
quick hack I installed yesterday.
With this change, HeapTupleData.t_datamcxt is no longer needed; will
remove it in a separate commit in HEAD only.
This commit is contained in:
Tom Lane 2005-11-20 18:38:20 +00:00
parent 33a9af738d
commit 40314f2dac
4 changed files with 125 additions and 102 deletions

View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.200 2005/10/15 02:49:08 momjian Exp $
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.201 2005/11/20 18:38:20 tgl Exp $
*
*
* INTERFACE ROUTINES
@ -1085,12 +1085,19 @@ heap_get_latest_tid(Relation relation,
*
* use_fsm is passed directly to RelationGetBufferForTuple, which see for
* more info.
*
* The return value is the OID assigned to the tuple (either here or by the
* caller), or InvalidOid if no OID. The header fields of *tup are updated
* to match the stored tuple; in particular tup->t_self receives the actual
* TID where the tuple was stored. But note that any toasting of fields
* within the tuple data is NOT reflected into *tup.
*/
Oid
heap_insert(Relation relation, HeapTuple tup, CommandId cid,
bool use_wal, bool use_fsm)
{
TransactionId xid = GetCurrentTransactionId();
HeapTuple heaptup;
Buffer buffer;
if (relation->rd_rel->relhasoids)
@ -1128,19 +1135,24 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
/*
* If the new tuple is too big for storage or contains already toasted
* out-of-line attributes from some other relation, invoke the toaster.
*
* Note: below this point, heaptup is the data we actually intend to
* store into the relation; tup is the caller's original untoasted data.
*/
if (HeapTupleHasExternal(tup) ||
(MAXALIGN(tup->t_len) > TOAST_TUPLE_THRESHOLD))
heap_tuple_toast_attrs(relation, tup, NULL);
heaptup = toast_insert_or_update(relation, tup, NULL);
else
heaptup = tup;
/* Find buffer to insert this tuple into */
buffer = RelationGetBufferForTuple(relation, tup->t_len,
buffer = RelationGetBufferForTuple(relation, heaptup->t_len,
InvalidBuffer, use_fsm);
/* NO EREPORT(ERROR) from here till changes are logged */
START_CRIT_SECTION();
RelationPutHeapTuple(relation, buffer, tup);
RelationPutHeapTuple(relation, buffer, heaptup);
/* XLOG stuff */
if (relation->rd_istemp)
@ -1158,15 +1170,15 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
uint8 info = XLOG_HEAP_INSERT;
xlrec.target.node = relation->rd_node;
xlrec.target.tid = tup->t_self;
xlrec.target.tid = heaptup->t_self;
rdata[0].data = (char *) &xlrec;
rdata[0].len = SizeOfHeapInsert;
rdata[0].buffer = InvalidBuffer;
rdata[0].next = &(rdata[1]);
xlhdr.t_natts = tup->t_data->t_natts;
xlhdr.t_infomask = tup->t_data->t_infomask;
xlhdr.t_hoff = tup->t_data->t_hoff;
xlhdr.t_natts = heaptup->t_data->t_natts;
xlhdr.t_infomask = heaptup->t_data->t_infomask;
xlhdr.t_hoff = heaptup->t_data->t_hoff;
/*
* note we mark rdata[1] as belonging to buffer; if XLogInsert decides
@ -1180,8 +1192,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
rdata[1].next = &(rdata[2]);
/* PG73FORMAT: write bitmap [+ padding] [+ oid] + data */
rdata[2].data = (char *) tup->t_data + offsetof(HeapTupleHeaderData, t_bits);
rdata[2].len = tup->t_len - offsetof(HeapTupleHeaderData, t_bits);
rdata[2].data = (char *) heaptup->t_data + offsetof(HeapTupleHeaderData, t_bits);
rdata[2].len = heaptup->t_len - offsetof(HeapTupleHeaderData, t_bits);
rdata[2].buffer = buffer;
rdata[2].buffer_std = true;
rdata[2].next = NULL;
@ -1191,7 +1203,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* page instead of restoring the whole thing. Set flag, and hide
* buffer references from XLogInsert.
*/
if (ItemPointerGetOffsetNumber(&(tup->t_self)) == FirstOffsetNumber &&
if (ItemPointerGetOffsetNumber(&(heaptup->t_self)) == FirstOffsetNumber &&
PageGetMaxOffsetNumber(page) == FirstOffsetNumber)
{
info |= XLOG_HEAP_INIT_PAGE;
@ -1212,13 +1224,23 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
/*
* If tuple is cachable, mark it for invalidation from the caches in case
* we abort. Note it is OK to do this after WriteBuffer releases the
* buffer, because the "tup" data structure is all in local memory, not in
* the shared buffer.
* buffer, because the heaptup data structure is all in local memory,
* not in the shared buffer.
*/
CacheInvalidateHeapTuple(relation, tup);
CacheInvalidateHeapTuple(relation, heaptup);
pgstat_count_heap_insert(&relation->pgstat_info);
/*
* If heaptup is a private copy, release it. Don't forget to copy t_self
* back to the caller's image, too.
*/
if (heaptup != tup)
{
tup->t_self = heaptup->t_self;
heap_freetuple(heaptup);
}
return HeapTupleGetOid(tup);
}
@ -1469,7 +1491,7 @@ l1:
* context lock on the buffer first.
*/
if (HeapTupleHasExternal(&tp))
heap_tuple_toast_attrs(relation, NULL, &tp);
toast_delete(relation, &tp);
/*
* Mark tuple for invalidation from system caches at next command
@ -1553,8 +1575,10 @@ simple_heap_delete(Relation relation, ItemPointer tid)
* HeapTupleSelfUpdated, HeapTupleUpdated, or HeapTupleBeingUpdated
* (the last only possible if wait == false).
*
* On success, newtup->t_self is set to the TID where the new tuple
* was inserted.
* On success, the header fields of *newtup are updated to match the new
* stored tuple; in particular, newtup->t_self is set to the TID where the
* new tuple was inserted. However, any TOAST changes in the new tuple's
* data are not reflected into *newtup.
*
* In the failure cases, the routine returns the tuple's t_ctid and t_xmax.
* If t_ctid is the same as otid, the tuple was deleted; if different, the
@ -1570,6 +1594,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
TransactionId xid = GetCurrentTransactionId();
ItemId lp;
HeapTupleData oldtup;
HeapTuple heaptup;
PageHeader dp;
Buffer buffer,
newbuf;
@ -1760,11 +1785,12 @@ l2:
* We need to invoke the toaster if there are already any out-of-line toasted
* values present, or if the new tuple is over-threshold.
*/
newtupsize = MAXALIGN(newtup->t_len);
need_toast = (HeapTupleHasExternal(&oldtup) ||
HeapTupleHasExternal(newtup) ||
(MAXALIGN(newtup->t_len) > TOAST_TUPLE_THRESHOLD));
newtupsize > TOAST_TUPLE_THRESHOLD);
newtupsize = MAXALIGN(newtup->t_len);
pagefree = PageGetFreeSpace((Page) dp);
if (need_toast || newtupsize > pagefree)
@ -1776,15 +1802,25 @@ l2:
HEAP_MOVED);
HeapTupleHeaderSetXmax(oldtup.t_data, xid);
HeapTupleHeaderSetCmax(oldtup.t_data, cid);
/* temporarily make it look not-updated */
oldtup.t_data->t_ctid = oldtup.t_self;
already_marked = true;
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
/* Let the toaster do its thing */
/*
* Let the toaster do its thing, if needed.
*
* Note: below this point, heaptup is the data we actually intend to
* store into the relation; newtup is the caller's original untoasted
* data.
*/
if (need_toast)
{
heap_tuple_toast_attrs(relation, newtup, &oldtup);
newtupsize = MAXALIGN(newtup->t_len);
heaptup = toast_insert_or_update(relation, newtup, &oldtup);
newtupsize = MAXALIGN(heaptup->t_len);
}
else
heaptup = newtup;
/*
* Now, do we need a new page for the tuple, or not? This is a bit
@ -1805,8 +1841,8 @@ l2:
*/
if (newtupsize > pagefree)
{
/* Assume there's no chance to put newtup on same page. */
newbuf = RelationGetBufferForTuple(relation, newtup->t_len,
/* Assume there's no chance to put heaptup on same page. */
newbuf = RelationGetBufferForTuple(relation, heaptup->t_len,
buffer, true);
}
else
@ -1823,7 +1859,7 @@ l2:
* seldom be taken.
*/
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
newbuf = RelationGetBufferForTuple(relation, newtup->t_len,
newbuf = RelationGetBufferForTuple(relation, heaptup->t_len,
buffer, true);
}
else
@ -1838,6 +1874,7 @@ l2:
/* No TOAST work needed, and it'll fit on same page */
already_marked = false;
newbuf = buffer;
heaptup = newtup;
}
/*
@ -1849,7 +1886,7 @@ l2:
/* NO EREPORT(ERROR) from here till changes are logged */
START_CRIT_SECTION();
RelationPutHeapTuple(relation, newbuf, newtup); /* insert new tuple */
RelationPutHeapTuple(relation, newbuf, heaptup); /* insert new tuple */
if (!already_marked)
{
@ -1863,13 +1900,13 @@ l2:
}
/* record address of new tuple in t_ctid of old one */
oldtup.t_data->t_ctid = newtup->t_self;
oldtup.t_data->t_ctid = heaptup->t_self;
/* XLOG stuff */
if (!relation->rd_istemp)
{
XLogRecPtr recptr = log_heap_update(relation, buffer, oldtup.t_self,
newbuf, newtup, false);
newbuf, heaptup, false);
if (newbuf != buffer)
{
@ -1905,10 +1942,10 @@ l2:
/*
* If new tuple is cachable, mark it for invalidation from the caches in
* case we abort. Note it is OK to do this after WriteBuffer releases the
* buffer, because the "newtup" data structure is all in local memory, not
* buffer, because the heaptup data structure is all in local memory, not
* in the shared buffer.
*/
CacheInvalidateHeapTuple(relation, newtup);
CacheInvalidateHeapTuple(relation, heaptup);
/*
* Release the lmgr tuple lock, if we had it.
@ -1918,6 +1955,16 @@ l2:
pgstat_count_heap_update(&relation->pgstat_info);
/*
* If heaptup is a private copy, release it. Don't forget to copy t_self
* back to the caller's image, too.
*/
if (heaptup != newtup)
{
newtup->t_self = heaptup->t_self;
heap_freetuple(heaptup);
}
return HeapTupleMayBeUpdated;
}

View File

@ -8,14 +8,17 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.53 2005/10/15 02:49:09 momjian Exp $
* $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.54 2005/11/20 18:38:20 tgl Exp $
*
*
* INTERFACE ROUTINES
* heap_tuple_toast_attrs -
* toast_insert_or_update -
* Try to make a given tuple fit into one page by compressing
* or moving off attributes
*
* toast_delete -
* Reclaim toast storage when a tuple is deleted
*
* heap_tuple_untoast_attr -
* Fetch back a given value from the "secondary" relation
*
@ -40,34 +43,13 @@
#undef TOAST_DEBUG
static void toast_delete(Relation rel, HeapTuple oldtup);
static void toast_delete_datum(Relation rel, Datum value);
static void toast_insert_or_update(Relation rel, HeapTuple newtup,
HeapTuple oldtup);
static Datum toast_save_datum(Relation rel, Datum value);
static varattrib *toast_fetch_datum(varattrib *attr);
static varattrib *toast_fetch_datum_slice(varattrib *attr,
int32 sliceoffset, int32 length);
/* ----------
* heap_tuple_toast_attrs -
*
* This is the central public entry point for toasting from heapam.
*
* Calls the appropriate event specific action.
* ----------
*/
void
heap_tuple_toast_attrs(Relation rel, HeapTuple newtup, HeapTuple oldtup)
{
if (newtup == NULL)
toast_delete(rel, oldtup);
else
toast_insert_or_update(rel, newtup, oldtup);
}
/* ----------
* heap_tuple_fetch_attr -
*
@ -305,7 +287,7 @@ toast_datum_size(Datum value)
* Cascaded delete toast-entries on DELETE
* ----------
*/
static void
void
toast_delete(Relation rel, HeapTuple oldtup)
{
TupleDesc tupleDesc;
@ -355,11 +337,22 @@ toast_delete(Relation rel, HeapTuple oldtup)
*
* Delete no-longer-used toast-entries and create new ones to
* make the new tuple fit on INSERT or UPDATE
*
* Inputs:
* newtup: the candidate new tuple to be inserted
* oldtup: the old row version for UPDATE, or NULL for INSERT
* Result:
* either newtup if no toasting is needed, or a palloc'd modified tuple
* that is what should actually get stored
*
* NOTE: neither newtup nor oldtup will be modified. This is a change
* from the pre-8.1 API of this routine.
* ----------
*/
static void
HeapTuple
toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
{
HeapTuple result_tuple;
TupleDesc tupleDesc;
Form_pg_attribute *att;
int numAttrs;
@ -757,7 +750,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
if (need_change)
{
HeapTupleHeader olddata = newtup->t_data;
char *new_data;
HeapTupleHeader new_data;
int32 new_len;
/*
@ -775,14 +768,18 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
toast_values, toast_isnull);
/*
* Allocate new tuple in same context as old one.
* Allocate and zero the space needed, and fill HeapTupleData fields.
*/
new_data = (char *) MemoryContextAlloc(newtup->t_datamcxt, new_len);
newtup->t_data = (HeapTupleHeader) new_data;
newtup->t_len = new_len;
result_tuple = (HeapTuple) palloc0(HEAPTUPLESIZE + new_len);
result_tuple->t_len = new_len;
result_tuple->t_self = newtup->t_self;
result_tuple->t_tableOid = newtup->t_tableOid;
result_tuple->t_datamcxt = CurrentMemoryContext;
new_data = (HeapTupleHeader) ((char *) result_tuple + HEAPTUPLESIZE);
result_tuple->t_data = new_data;
/*
* Put the tuple header and the changed values into place
* Put the existing tuple header and the changed values into place
*/
memcpy(new_data, olddata, olddata->t_hoff);
@ -790,16 +787,11 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
toast_values,
toast_isnull,
(char *) new_data + olddata->t_hoff,
&(newtup->t_data->t_infomask),
has_nulls ? newtup->t_data->t_bits : NULL);
/*
* In the case we modified a previously modified tuple again, free the
* memory from the previous run
*/
if ((char *) olddata != ((char *) newtup + HEAPTUPLESIZE))
pfree(olddata);
&(new_data->t_infomask),
has_nulls ? new_data->t_bits : NULL);
}
else
result_tuple = newtup;
/*
* Free allocated temp values
@ -816,6 +808,8 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
for (i = 0; i < numAttrs; i++)
if (toast_delold[i])
toast_delete_datum(rel, toast_oldvalues[i]);
return result_tuple;
}

View File

@ -26,7 +26,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.259 2005/11/19 20:57:44 tgl Exp $
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.260 2005/11/20 18:38:20 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -1445,16 +1445,6 @@ ExecInsert(TupleTableSlot *slot,
estate->es_lastoid = newId;
setLastTid(&(tuple->t_self));
/*
* KLUGE SOLUTION for bug found post 8.1 release: if the tuple toaster
* fired on the tuple then it changed the physical tuple inside the
* tuple slot, leaving any extracted information invalid. Mark the
* extracted state invalid just in case. Need to fix things so that
* the toaster gets to run against the tuple before we materialize it,
* but that's way too invasive for a stable branch.
*/
slot->tts_nvalid = 0;
/*
* insert index entries for tuple
*/
@ -1709,16 +1699,6 @@ lreplace:;
IncrReplaced();
(estate->es_processed)++;
/*
* KLUGE SOLUTION for bug found post 8.1 release: if the tuple toaster
* fired on the tuple then it changed the physical tuple inside the
* tuple slot, leaving any extracted information invalid. Mark the
* extracted state invalid just in case. Need to fix things so that
* the toaster gets to run against the tuple before we materialize it,
* but that's way too invasive for a stable branch.
*/
slot->tts_nvalid = 0;
/*
* Note: instead of having to update the old index tuples associated with
* the heap tuple, all we do is form and insert new index tuples. This is

View File

@ -6,7 +6,7 @@
*
* Copyright (c) 2000-2005, PostgreSQL Global Development Group
*
* $PostgreSQL: pgsql/src/include/access/tuptoaster.h,v 1.23 2005/07/06 19:02:53 momjian Exp $
* $PostgreSQL: pgsql/src/include/access/tuptoaster.h,v 1.24 2005/11/20 18:38:20 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -66,19 +66,21 @@
/* ----------
* heap_tuple_toast_attrs() -
* toast_insert_or_update -
*
* Called by heap_insert(), heap_update() and heap_delete().
* Outdates any no-longer-needed toast entries referenced
* by oldtup and creates new ones until newtup is no more than
* TOAST_TUPLE_TARGET (or we run out of toastable values).
* Possibly modifies newtup by replacing the t_data part!
*
* oldtup is NULL if insert, newtup is NULL if delete.
* Called by heap_insert() and heap_update().
* ----------
*/
extern void heap_tuple_toast_attrs(Relation rel,
HeapTuple newtup, HeapTuple oldtup);
extern HeapTuple toast_insert_or_update(Relation rel,
HeapTuple newtup, HeapTuple oldtup);
/* ----------
* toast_delete -
*
* Called by heap_delete().
* ----------
*/
extern void toast_delete(Relation rel, HeapTuple oldtup);
/* ----------
* heap_tuple_fetch_attr() -