From 40314f2dac2ecb2974d03c064917a70de74c63d5 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 20 Nov 2005 18:38:20 +0000
Subject: [PATCH] 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.
---
src/backend/access/heap/heapam.c | 107 +++++++++++++++++++--------
src/backend/access/heap/tuptoaster.c | 74 +++++++++---------
src/backend/executor/execMain.c | 22 +-----
src/include/access/tuptoaster.h | 22 +++---
4 files changed, 124 insertions(+), 101 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 6c669ed62b4..fc750884c75 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -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;
}
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index fd20f111b80..99d725c27cc 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -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;
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6f79c4d58b0..c0c6cfbe500 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -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
diff --git a/src/include/access/tuptoaster.h b/src/include/access/tuptoaster.h
index b45df277b1e..b5870e69dcb 100644
--- a/src/include/access/tuptoaster.h
+++ b/src/include/access/tuptoaster.h
@@ -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!
+ * Called by heap_insert() and heap_update().
+ * ----------
+ */
+extern HeapTuple toast_insert_or_update(Relation rel,
+ HeapTuple newtup, HeapTuple oldtup);
+
+/* ----------
+ * toast_delete -
*
- * oldtup is NULL if insert, newtup is NULL if delete.
+ * Called by heap_delete().
* ----------
*/
-extern void heap_tuple_toast_attrs(Relation rel,
- HeapTuple newtup, HeapTuple oldtup);
+extern void toast_delete(Relation rel, HeapTuple oldtup);
/* ----------
* heap_tuple_fetch_attr() -
--
GitLab