Decouple the values of TOAST_TUPLE_THRESHOLD and TOAST_MAX_CHUNK_SIZE.

Add the latter to the values checked in pg_control, since it can't be changed
without invalidating toast table content.  This commit in itself shouldn't
change any behavior, but it lays some necessary groundwork for experimentation
with these toast-control numbers.

Note: while TOAST_TUPLE_THRESHOLD can now be changed without initdb, some
thought still needs to be given to needs_toast_table() in toasting.c before
unleashing random changes.

7 files changed, 105 insertions, 42 deletions

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a99aa4ced0a..f561e351f22 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.230 2007/03/29 00:15:37 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.231 2007/04/03 04:14:26 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1420,7 +1420,13 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 	 * 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) || tup->t_len > TOAST_TUPLE_THRESHOLD)
+	if (relation->rd_rel->relkind == RELKIND_TOASTVALUE)
+	{
+		/* toast table entries should never be recursively toasted */
+		Assert(!HeapTupleHasExternal(tup));
+		heaptup = tup;
+	}
+	else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
 		heaptup = toast_insert_or_update(relation, tup, NULL,
 										 use_wal, use_fsm);
 	else
@@ -1777,7 +1783,12 @@ l1:
 	 * because we need to look at the contents of the tuple, but it's OK to
 	 * release the content lock on the buffer first.
 	 */
-	if (HeapTupleHasExternal(&tp))
+	if (relation->rd_rel->relkind == RELKIND_TOASTVALUE)
+	{
+		/* toast table entries should never be recursively toasted */
+		Assert(!HeapTupleHasExternal(&tp));
+	}
+	else if (HeapTupleHasExternal(&tp))
 		toast_delete(relation, &tp);
 
 	/*
@@ -2075,9 +2086,17 @@ 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.
 	 */
-	need_toast = (HeapTupleHasExternal(&oldtup) ||
-				  HeapTupleHasExternal(newtup) ||
-				  newtup->t_len > TOAST_TUPLE_THRESHOLD);
+	if (relation->rd_rel->relkind == RELKIND_TOASTVALUE)
+	{
+		/* toast table entries should never be recursively toasted */
+		Assert(!HeapTupleHasExternal(&oldtup));
+		Assert(!HeapTupleHasExternal(newtup));
+		need_toast = false;
+	}
+	else
+		need_toast = (HeapTupleHasExternal(&oldtup) ||
+					  HeapTupleHasExternal(newtup) ||
+					  newtup->t_len > TOAST_TUPLE_THRESHOLD);
 
 	pagefree = PageGetFreeSpace((Page) dp);
 
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index b1e02e13755..334d6700423 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.72 2007/03/29 00:15:37 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.73 2007/04/03 04:14:26 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -292,6 +292,12 @@ toast_delete(Relation rel, HeapTuple oldtup)
 	bool		toast_isnull[MaxHeapAttributeNumber];
 
 	/*
+	 * We should only ever be called for tuples of plain relations ---
+	 * recursing on a toast rel is bad news.
+	 */
+	Assert(rel->rd_rel->relkind == RELKIND_RELATION);
+
+	/*
 	 * Get the tuple descriptor and break down the tuple into fields.
 	 *
 	 * NOTE: it's debatable whether to use heap_deformtuple() here or just
@@ -360,6 +366,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 	bool		has_nulls = false;
 
 	Size		maxDataLen;
+	Size		hoff;
 
 	char		toast_action[MaxHeapAttributeNumber];
 	bool		toast_isnull[MaxHeapAttributeNumber];
@@ -371,6 +378,12 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 	bool		toast_delold[MaxHeapAttributeNumber];
 
 	/*
+	 * We should only ever be called for tuples of plain relations ---
+	 * recursing on a toast rel is bad news.
+	 */
+	Assert(rel->rd_rel->relkind == RELKIND_RELATION);
+
+	/*
 	 * Get the tuple descriptor and break down the tuple(s) into fields.
 	 */
 	tupleDesc = rel->rd_att;
@@ -512,15 +525,15 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 	 */
 
 	/* compute header overhead --- this should match heap_form_tuple() */
-	maxDataLen = offsetof(HeapTupleHeaderData, t_bits);
+	hoff = offsetof(HeapTupleHeaderData, t_bits);
 	if (has_nulls)
-		maxDataLen += BITMAPLEN(numAttrs);
+		hoff += BITMAPLEN(numAttrs);
 	if (newtup->t_data->t_infomask & HEAP_HASOID)
-		maxDataLen += sizeof(Oid);
-	maxDataLen = MAXALIGN(maxDataLen);
-	Assert(maxDataLen == newtup->t_data->t_hoff);
+		hoff += sizeof(Oid);
+	hoff = MAXALIGN(hoff);
+	Assert(hoff == newtup->t_data->t_hoff);
 	/* now convert to a limit on the tuple data size */
-	maxDataLen = TOAST_TUPLE_TARGET - maxDataLen;
+	maxDataLen = TOAST_TUPLE_TARGET - hoff;
 
 	/*
 	 * Look for attributes with attstorage 'x' to compress
@@ -583,7 +596,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 
 	/*
 	 * Second we look for attributes of attstorage 'x' or 'e' that are still
-	 * inline.
+	 * inline.  But skip this if there's no toast table to push them to.
 	 */
 	while (heap_compute_data_size(tupleDesc,
 								  toast_values, toast_isnull) > maxDataLen &&
@@ -695,7 +708,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
 	}
 
 	/*
-	 * Finally we store attributes of type 'm' external
+	 * Finally we store attributes of type 'm' external, if possible.
 	 */
 	while (heap_compute_data_size(tupleDesc,
 								  toast_values, toast_isnull) > maxDataLen &&
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 07e95057d0e..f3e02dd6316 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.265 2007/03/03 20:02:26 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.266 2007/04/03 04:14:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,6 +28,7 @@
 #include "access/multixact.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
+#include "access/tuptoaster.h"
 #include "access/twophase.h"
 #include "access/xact.h"
 #include "access/xlog_internal.h"
@@ -3634,6 +3635,8 @@ WriteControlFile(void)
 	ControlFile->nameDataLen = NAMEDATALEN;
 	ControlFile->indexMaxKeys = INDEX_MAX_KEYS;
 
+	ControlFile->toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE;
+
 #ifdef HAVE_INT64_TIMESTAMP
 	ControlFile->enableIntTimes = TRUE;
 #else
@@ -3824,6 +3827,13 @@ ReadControlFile(void)
 					  " but the server was compiled with INDEX_MAX_KEYS %d.",
 						   ControlFile->indexMaxKeys, INDEX_MAX_KEYS),
 				 errhint("It looks like you need to recompile or initdb.")));
+	if (ControlFile->toast_max_chunk_size != TOAST_MAX_CHUNK_SIZE)
+		ereport(FATAL,
+				(errmsg("database files are incompatible with server"),
+				 errdetail("The database cluster was initialized with TOAST_MAX_CHUNK_SIZE %d,"
+						   " but the server was compiled with TOAST_MAX_CHUNK_SIZE %d.",
+						   ControlFile->toast_max_chunk_size, (int) TOAST_MAX_CHUNK_SIZE),
+				 errhint("It looks like you need to recompile or initdb.")));
 
 #ifdef HAVE_INT64_TIMESTAMP
 	if (ControlFile->enableIntTimes != TRUE)
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index 4b6c2f5f961..6538da6a56d 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -6,7 +6,7 @@
  * copyright (c) Oliver Elphick <olly@lfix.co.uk>, 2001;
  * licence: BSD
  *
- * $PostgreSQL: pgsql/src/bin/pg_controldata/pg_controldata.c,v 1.34 2007/03/18 16:50:43 neilc Exp $
+ * $PostgreSQL: pgsql/src/bin/pg_controldata/pg_controldata.c,v 1.35 2007/04/03 04:14:26 tgl Exp $
  */
 #include "postgres.h"
 
@@ -199,6 +199,8 @@ main(int argc, char *argv[])
 		   ControlFile.nameDataLen);
 	printf(_("Maximum columns in an index:          %u\n"),
 		   ControlFile.indexMaxKeys);
+	printf(_("Maximum size of a TOAST chunk:        %u\n"),
+		   ControlFile.toast_max_chunk_size);
 	printf(_("Date/time type storage:               %s\n"),
 		   (ControlFile.enableIntTimes ? _("64-bit integers") : _("floating-point numbers")));
 	printf(_("Maximum length of locale name:        %u\n"),
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
index c272fc432d0..c0a48128b26 100644
--- a/src/bin/pg_resetxlog/pg_resetxlog.c
+++ b/src/bin/pg_resetxlog/pg_resetxlog.c
@@ -23,7 +23,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/bin/pg_resetxlog/pg_resetxlog.c,v 1.58 2007/03/03 20:02:27 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/pg_resetxlog/pg_resetxlog.c,v 1.59 2007/04/03 04:14:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -41,6 +41,7 @@
 #endif
 
 #include "access/transam.h"
+#include "access/tuptoaster.h"
 #include "access/multixact.h"
 #include "access/xlog_internal.h"
 #include "catalog/catversion.h"
@@ -484,6 +485,7 @@ GuessControlValues(void)
 	ControlFile.xlog_seg_size = XLOG_SEG_SIZE;
 	ControlFile.nameDataLen = NAMEDATALEN;
 	ControlFile.indexMaxKeys = INDEX_MAX_KEYS;
+	ControlFile.toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE;
 #ifdef HAVE_INT64_TIMESTAMP
 	ControlFile.enableIntTimes = TRUE;
 #else
@@ -572,6 +574,8 @@ PrintControlValues(bool guessed)
 		   ControlFile.nameDataLen);
 	printf(_("Maximum columns in an index:          %u\n"),
 		   ControlFile.indexMaxKeys);
+	printf(_("Maximum size of a TOAST chunk:        %u\n"),
+		   ControlFile.toast_max_chunk_size);
 	printf(_("Date/time type storage:               %s\n"),
 		   (ControlFile.enableIntTimes ? _("64-bit integers") : _("floating-point numbers")));
 	printf(_("Maximum length of locale name:        %u\n"),
diff --git a/src/include/access/tuptoaster.h b/src/include/access/tuptoaster.h
index 6cc0bdcbe8c..3ab29979468 100644
--- a/src/include/access/tuptoaster.h
+++ b/src/include/access/tuptoaster.h
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 2000-2007, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/access/tuptoaster.h,v 1.33 2007/03/29 00:15:39 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/access/tuptoaster.h,v 1.34 2007/04/03 04:14:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,19 +29,26 @@
  * TOAST_TUPLE_TARGET bytes.  Both numbers include all tuple header overhead
  * and between-fields alignment padding, but we do *not* consider any
  * end-of-tuple alignment padding; hence the values can be compared directly
- * to a tuple's t_len field.  We choose TOAST_TUPLE_THRESHOLD with the
- * knowledge that toast-table tuples will be exactly that size, and we'd
- * like to fit four of them per page with minimal space wastage.
+ * to a tuple's t_len field.
  *
- * The numbers need not be the same, though they currently are.
+ * The numbers need not be the same, though they currently are.  It doesn't
+ * make sense for TARGET to exceed THRESHOLD, but it could be useful to make
+ * it be smaller.
  *
- * Note: sizeof(PageHeaderData) includes the first ItemId, but we have
- * to allow for 3 more, if we want to fit 4 tuples on a page.
+ * Currently we choose both values to match the largest tuple size for which
+ * TOAST_TUPLES_PER_PAGE tuples can fit on a disk page.
+ *
+ * XXX while these can be modified without initdb, some thought needs to be
+ * given to needs_toast_table() in toasting.c before unleashing random
+ * changes.
  */
+#define TOAST_TUPLES_PER_PAGE	4
+
+/* Note: sizeof(PageHeaderData) includes the first ItemId on the page */
 #define TOAST_TUPLE_THRESHOLD	\
 	MAXALIGN_DOWN((BLCKSZ - \
-				   MAXALIGN(sizeof(PageHeaderData) + 3 * sizeof(ItemIdData))) \
-				  / 4)
+				   MAXALIGN(sizeof(PageHeaderData) + (TOAST_TUPLES_PER_PAGE-1) * sizeof(ItemIdData))) \
+				  / TOAST_TUPLES_PER_PAGE)
 
 #define TOAST_TUPLE_TARGET		TOAST_TUPLE_THRESHOLD
 
@@ -56,20 +63,26 @@
  * When we store an oversize datum externally, we divide it into chunks
  * containing at most TOAST_MAX_CHUNK_SIZE data bytes.	This number *must*
  * be small enough that the completed toast-table tuple (including the
- * ID and sequence fields and all overhead) is no more than MaxHeapTupleSize
- * bytes.  It *should* be small enough to make toast-table tuples no more
- * than TOAST_TUPLE_THRESHOLD bytes, else heapam.c will uselessly invoke
- * the toaster on toast-table tuples.  The current coding ensures that the
- * maximum tuple length is exactly TOAST_TUPLE_THRESHOLD bytes.
- *
- * NB: you cannot change this value without forcing initdb, at least not
- * if your DB contains any multi-chunk toasted values.
+ * ID and sequence fields and all overhead) will fit on a page.
+ * The coding here sets the size on the theory that we want to fit
+ * EXTERN_TUPLES_PER_PAGE tuples of maximum size onto a page.
+ *
+ * NB: Changing TOAST_MAX_CHUNK_SIZE requires an initdb.
  */
-#define TOAST_MAX_CHUNK_SIZE	(TOAST_TUPLE_THRESHOLD -			\
-				MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) -	\
-				sizeof(Oid) -										\
-				sizeof(int32) -										\
-				VARHDRSZ)
+#define EXTERN_TUPLES_PER_PAGE	4				/* tweak only this */
+
+/* Note: sizeof(PageHeaderData) includes the first ItemId on the page */
+#define EXTERN_TUPLE_MAX_SIZE	\
+	MAXALIGN_DOWN((BLCKSZ - \
+				   MAXALIGN(sizeof(PageHeaderData) + (EXTERN_TUPLES_PER_PAGE-1) * sizeof(ItemIdData))) \
+				  / EXTERN_TUPLES_PER_PAGE)
+
+#define TOAST_MAX_CHUNK_SIZE	\
+	(EXTERN_TUPLE_MAX_SIZE -							\
+	 MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) -	\
+	 sizeof(Oid) -										\
+	 sizeof(int32) -									\
+	 VARHDRSZ)
 
 
 /* ----------
diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index ce4e5c2dc52..23c2681a8fb 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/pg_control.h,v 1.36 2007/03/03 20:02:27 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/pg_control.h,v 1.37 2007/04/03 04:14:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -22,7 +22,7 @@
 
 
 /* Version identifier for this pg_control format */
-#define PG_CONTROL_VERSION	832
+#define PG_CONTROL_VERSION	833
 
 /*
  * Body of CheckPoint XLOG records.  This is declared here because we keep
@@ -135,6 +135,8 @@ typedef struct ControlFileData
 	uint32		nameDataLen;	/* catalog name field width */
 	uint32		indexMaxKeys;	/* max number of columns in an index */
 
+	uint32		toast_max_chunk_size;	/* chunk size in TOAST tables */
+
 	/* flag indicating internal format of timestamp, interval, time */
 	uint32		enableIntTimes; /* int64 storage enabled? */