Fix portability problems recently exposed by regression tests on Alphas.

1. Distinguish cases where a Datum representing a tuple datatype is an OID
from cases where it is a pointer to TupleTableSlot, and make sure we use
the right typlen in each case.
2. Make fetchatt() and related code support 8-byte by-value datatypes on
machines where Datum is 8 bytes.  Centralize knowledge of the available
by-value datatype sizes in two macros in tupmacs.h, so that this will be
easier if we ever have to do it again.

4 files changed, 168 insertions, 203 deletions

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index e76fa5a5652..6405320f85d 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -2,14 +2,14 @@
  *
  * heaptuple.c
  *	  This file contains heap tuple accessor and mutator routines, as well
- *	  as a few various tuple utilities.
+ *	  as various tuple utilities.
  *
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/heaptuple.c,v 1.67 2000/11/30 18:38:45 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/heaptuple.c,v 1.68 2000/12/27 23:59:10 tgl Exp $
  *
  * NOTES
  *	  The old interface functions have been converted to macros
@@ -23,16 +23,6 @@
 #include "access/heapam.h"
 #include "catalog/pg_type.h"
 
-/* Used by heap_getattr() macro, for speed */
-long		heap_sysoffset[] = {
-/* Only the first one is pass-by-ref, and is handled specially in the macro */
-	offsetof(HeapTupleHeaderData, t_ctid),
-	offsetof(HeapTupleHeaderData, t_oid),
-	offsetof(HeapTupleHeaderData, t_xmin),
-	offsetof(HeapTupleHeaderData, t_cmin),
-	offsetof(HeapTupleHeaderData, t_xmax),
-	offsetof(HeapTupleHeaderData, t_cmax)
-};
 
 /* ----------------------------------------------------------------
  *						misc support routines
@@ -48,12 +38,12 @@ ComputeDataSize(TupleDesc tupleDesc,
 				Datum *value,
 				char *nulls)
 {
-	uint32		data_length;
+	uint32		data_length = 0;
 	int			i;
 	int			numberOfAttributes = tupleDesc->natts;
 	Form_pg_attribute *att = tupleDesc->attrs;
 
-	for (data_length = 0, i = 0; i < numberOfAttributes; i++)
+	for (i = 0; i < numberOfAttributes; i++)
 	{
 		if (nulls[i] != ' ')
 			continue;
@@ -114,38 +104,33 @@ DataFill(char *data,
 			*bitP |= bitmask;
 		}
 
+		/* XXX we are aligning the pointer itself, not the offset */
 		data = (char *) att_align((long) data, att[i]->attlen, att[i]->attalign);
-		switch (att[i]->attlen)
+
+		if (att[i]->attbyval)
 		{
-			case -1:
-				*infomask |= HEAP_HASVARLENA;
-				if (VARATT_IS_EXTERNAL(value[i]))
-					*infomask |= HEAP_HASEXTERNAL;
-				if (VARATT_IS_COMPRESSED(value[i]))
-					*infomask |= HEAP_HASCOMPRESSED;
-				data_length = VARATT_SIZE(DatumGetPointer(value[i]));
-				memmove(data, DatumGetPointer(value[i]), data_length);
-				break;
-			case sizeof(char):
-				*data = att[i]->attbyval ?
-					DatumGetChar(value[i]) : *((char *) value[i]);
-				break;
-			case sizeof(int16):
-				*(short *) data = (att[i]->attbyval ?
-								   DatumGetInt16(value[i]) :
-								   *((short *) value[i]));
-				break;
-			case sizeof(int32):
-				*(int32 *) data = (att[i]->attbyval ?
-								   DatumGetInt32(value[i]) :
-								   *((int32 *) value[i]));
-				break;
-			default:
-				Assert(att[i]->attlen >= 0);
-				memmove(data, DatumGetPointer(value[i]),
-						(size_t) (att[i]->attlen));
-				break;
+			/* pass-by-value */
+			store_att_byval(data, value[i], att[i]->attlen);
+		}
+		else if (att[i]->attlen == -1)
+		{
+			/* varlena */
+			*infomask |= HEAP_HASVARLENA;
+			if (VARATT_IS_EXTERNAL(value[i]))
+				*infomask |= HEAP_HASEXTERNAL;
+			if (VARATT_IS_COMPRESSED(value[i]))
+				*infomask |= HEAP_HASCOMPRESSED;
+			data_length = VARATT_SIZE(DatumGetPointer(value[i]));
+			memcpy(data, DatumGetPointer(value[i]), data_length);
+		}
+		else
+		{
+			/* fixed-length pass-by-reference */
+			Assert(att[i]->attlen >= 0);
+			memcpy(data, DatumGetPointer(value[i]),
+				   (size_t) (att[i]->attlen));
 		}
+
 		data = (char *) att_addlength((long) data, att[i]->attlen, value[i]);
 	}
 }
@@ -192,89 +177,6 @@ heap_attisnull(HeapTuple tup, int attnum)
 	return 0;
 }
 
-/* ----------------------------------------------------------------
- *				 system attribute heap tuple support
- * ----------------------------------------------------------------
- */
-
-/* ----------------
- *		heap_sysattrlen
- *
- *		This routine returns the length of a system attribute.
- * ----------------
- */
-int
-heap_sysattrlen(AttrNumber attno)
-{
-	HeapTupleHeader f = NULL;
-
-	switch (attno)
-	{
-		case TableOidAttributeNumber:
-			return sizeof f->t_oid;
-		case SelfItemPointerAttributeNumber:
-			return sizeof f->t_ctid;
-		case ObjectIdAttributeNumber:
-			return sizeof f->t_oid;
-		case MinTransactionIdAttributeNumber:
-			return sizeof f->t_xmin;
-		case MinCommandIdAttributeNumber:
-			return sizeof f->t_cmin;
-		case MaxTransactionIdAttributeNumber:
-			return sizeof f->t_xmax;
-		case MaxCommandIdAttributeNumber:
-			return sizeof f->t_cmax;
-
-		default:
-			elog(ERROR, "sysattrlen: System attribute number %d unknown.", attno);
-			return 0;
-	}
-}
-
-/* ----------------
- *		heap_sysattrbyval
- *
- *		This routine returns the "by-value" property of a system attribute.
- * ----------------
- */
-bool
-heap_sysattrbyval(AttrNumber attno)
-{
-	bool		byval;
-
-	switch (attno)
-	{
-		case TableOidAttributeNumber:
-			byval = true;
-			break;
-		case SelfItemPointerAttributeNumber:
-			byval = false;
-			break;
-		case ObjectIdAttributeNumber:
-			byval = true;
-			break;
-		case MinTransactionIdAttributeNumber:
-			byval = true;
-			break;
-		case MinCommandIdAttributeNumber:
-			byval = true;
-			break;
-		case MaxTransactionIdAttributeNumber:
-			byval = true;
-			break;
-		case MaxCommandIdAttributeNumber:
-			byval = true;
-			break;
-		default:
-			byval = true;
-			elog(ERROR, "sysattrbyval: System attribute number %d unknown.",
-				 attno);
-			break;
-	}
-
-	return byval;
-}
-
 /* ----------------
  *		nocachegetattr
  *
@@ -332,8 +234,7 @@ nocachegetattr(HeapTuple tuple,
 /* This is handled in the macro */
 		if (att[attnum]->attcacheoff != -1)
 		{
-			return (Datum)
-				fetchatt(&(att[attnum]),
+			return fetchatt(att[attnum],
 				  (char *) tup + tup->t_hoff + att[attnum]->attcacheoff);
 		}
 #endif
@@ -397,8 +298,8 @@ nocachegetattr(HeapTuple tuple,
 	{
 		if (att[attnum]->attcacheoff != -1)
 		{
-			return (Datum) fetchatt(&(att[attnum]),
-									tp + att[attnum]->attcacheoff);
+			return fetchatt(att[attnum],
+							tp + att[attnum]->attcacheoff);
 		}
 		else if (!HeapTupleAllFixed(tuple))
 		{
@@ -460,7 +361,7 @@ nocachegetattr(HeapTuple tuple,
 			off = att_addlength(off, att[j]->attlen, tp + off);
 		}
 
-		return (Datum) fetchatt(&(att[attnum]), tp + att[attnum]->attcacheoff);
+		return fetchatt(att[attnum], tp + att[attnum]->attcacheoff);
 	}
 	else
 	{
@@ -508,11 +409,67 @@ nocachegetattr(HeapTuple tuple,
 
 		off = att_align(off, att[attnum]->attlen, att[attnum]->attalign);
 
-		return (Datum) fetchatt(&(att[attnum]), tp + off);
+		return fetchatt(att[attnum], tp + off);
 	}
 }
 
 /* ----------------
+ *		heap_getsysattr
+ *
+ *		Fetch the value of a system attribute for a tuple.
+ *
+ * This is a support routine for the heap_getattr macro.  The macro
+ * has already determined that the attnum refers to a system attribute.
+ * ----------------
+ */
+Datum
+heap_getsysattr(HeapTuple tup, int attnum, bool *isnull)
+{
+	Datum		result;
+
+	Assert(tup);
+
+	/* Currently, no sys attribute ever reads as NULL. */
+	if (isnull)
+		*isnull = false;
+
+	switch (attnum)
+	{
+		case SelfItemPointerAttributeNumber:
+			/* pass-by-reference datatype */
+			result = PointerGetDatum(&(tup->t_self));
+			break;
+		case ObjectIdAttributeNumber:
+			result = ObjectIdGetDatum(tup->t_data->t_oid);
+			break;
+		case MinTransactionIdAttributeNumber:
+			/* XXX should have a TransactionIdGetDatum macro */
+			result = (Datum) (tup->t_data->t_xmin);
+			break;
+		case MinCommandIdAttributeNumber:
+			/* XXX should have a CommandIdGetDatum macro */
+			result = (Datum) (tup->t_data->t_cmin);
+			break;
+		case MaxTransactionIdAttributeNumber:
+			/* XXX should have a TransactionIdGetDatum macro */
+			result = (Datum) (tup->t_data->t_xmax);
+			break;
+		case MaxCommandIdAttributeNumber:
+			/* XXX should have a CommandIdGetDatum macro */
+			result = (Datum) (tup->t_data->t_cmax);
+			break;
+		case TableOidAttributeNumber:
+			result = ObjectIdGetDatum(tup->t_tableOid);
+			break;
+		default:
+			elog(ERROR, "heap_getsysattr: invalid attnum %d", attnum);
+			result = 0;			/* keep compiler quiet */
+			break;
+	}
+	return result;
+}
+
+/* ----------------
  *		heap_copytuple
  *
  *		returns a copy of an entire tuple
@@ -630,18 +587,21 @@ heap_formtuple(TupleDesc tupleDescriptor,
 	int			i;
 	int			numberOfAttributes = tupleDescriptor->natts;
 
+	if (numberOfAttributes > MaxHeapAttributeNumber)
+		elog(ERROR, "heap_formtuple: numberOfAttributes of %d > %d",
+			 numberOfAttributes, MaxHeapAttributeNumber);
+
 	len = offsetof(HeapTupleHeaderData, t_bits);
 
-	for (i = 0; i < numberOfAttributes && !hasnull; i++)
+	for (i = 0; i < numberOfAttributes; i++)
 	{
 		if (nulls[i] != ' ')
+		{
 			hasnull = true;
+			break;
+		}
 	}
 
-	if (numberOfAttributes > MaxHeapAttributeNumber)
-		elog(ERROR, "heap_formtuple: numberOfAttributes of %d > %d",
-			 numberOfAttributes, MaxHeapAttributeNumber);
-
 	if (hasnull)
 	{
 		bitmaplen = BITMAPLEN(numberOfAttributes);
diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index 837b7f632bb..d473ce6c94d 100644
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/indextuple.c,v 1.48 2000/12/07 02:00:47 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/indextuple.c,v 1.49 2000/12/27 23:59:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -217,9 +217,9 @@ nocache_index_getattr(IndexTuple tup,
 /* This is handled in the macro */
 		if (att[attnum]->attcacheoff != -1)
 		{
-			return (Datum) fetchatt(&(att[attnum]),
-									(char *) tup + data_off +
-									att[attnum]->attcacheoff);
+			return fetchatt(att[attnum],
+							(char *) tup + data_off +
+							att[attnum]->attcacheoff);
 		}
 #endif
 	}
@@ -279,8 +279,8 @@ nocache_index_getattr(IndexTuple tup,
 	{
 		if (att[attnum]->attcacheoff != -1)
 		{
-			return (Datum) fetchatt(&(att[attnum]),
-									tp + att[attnum]->attcacheoff);
+			return fetchatt(att[attnum],
+							tp + att[attnum]->attcacheoff);
 		}
 		else if (!IndexTupleAllFixed(tup))
 		{
@@ -332,7 +332,7 @@ nocache_index_getattr(IndexTuple tup,
 			off += att[j]->attlen;
 		}
 
-		return (Datum) fetchatt(&(att[attnum]), tp + att[attnum]->attcacheoff);
+		return fetchatt(att[attnum], tp + att[attnum]->attcacheoff);
 	}
 	else
 	{
@@ -379,7 +379,7 @@ nocache_index_getattr(IndexTuple tup,
 
 		off = att_align(off, att[attnum]->attlen, att[attnum]->attalign);
 
-		return (Datum) fetchatt(&att[attnum], tp + off);
+		return fetchatt(att[attnum], tp + off);
 	}
 }
 
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index b05902068ee..5b79cb84a48 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/printtup.c,v 1.55 2000/12/01 22:10:31 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/printtup.c,v 1.56 2000/12/27 23:59:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -436,30 +436,17 @@ printtup_internal(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
 			pq_sendint(&buf, len, sizeof(int32));
 			if (typeinfo->attrs[i]->attbyval)
 			{
-				int8		i8;
-				int16		i16;
-				int32		i32;
-
-				switch (len)
-				{
-					case sizeof(int8):
-						i8 = DatumGetChar(attr);
-						pq_sendbytes(&buf, (char *) &i8, len);
-						break;
-					case sizeof(int16):
-						i16 = DatumGetInt16(attr);
-						pq_sendbytes(&buf, (char *) &i16, len);
-						break;
-					case sizeof(int32):
-						i32 = DatumGetInt32(attr);
-						pq_sendbytes(&buf, (char *) &i32, len);
-						break;
-					default:
-						elog(ERROR, "printtup_internal: unexpected typlen");
-						break;
-				}
+				Datum		datumBuf;
+
+				/*
+				 * We need this horsing around because we don't know how
+				 * shorter data values are aligned within a Datum.
+				 */
+				store_att_byval(&datumBuf, attr, len);
+				pq_sendbytes(&buf, (char *) &datumBuf, len);
 #ifdef IPORTAL_DEBUG
-				fprintf(stderr, "byval length %d data %d\n", len, attr);
+				fprintf(stderr, "byval length %d data %ld\n", len,
+						(long) attr);
 #endif
 			}
 			else
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 8b9b7cd537f..83977d6a387 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.69 2000/11/16 22:30:15 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.70 2000/12/27 23:59:10 tgl Exp $
  *
  * NOTES
  *	  some of the executor utility code such as "ExecTypeFromTL" should be
@@ -352,7 +352,6 @@ TupleDescInitEntry(TupleDesc desc,
 
 	AssertArg(!PointerIsValid(desc->attrs[attributeNumber - 1]));
 
-
 	/* ----------------
 	 *	allocate storage for this attribute
 	 * ----------------
@@ -362,7 +361,7 @@ TupleDescInitEntry(TupleDesc desc,
 	desc->attrs[attributeNumber - 1] = att;
 
 	/* ----------------
-	 *	initialize some of the attribute fields
+	 *	initialize the attribute fields
 	 * ----------------
 	 */
 	att->attrelid = 0;			/* dummy value */
@@ -372,7 +371,6 @@ TupleDescInitEntry(TupleDesc desc,
 	else
 		MemSet(NameStr(att->attname), 0, NAMEDATALEN);
 
-
 	att->attdispersion = 0;		/* dummy value */
 	att->attcacheoff = -1;
 	att->atttypmod = typmod;
@@ -414,8 +412,8 @@ TupleDescInitEntry(TupleDesc desc,
 		att->atttypid = InvalidOid;
 		att->attlen = (int16) 0;
 		att->attbyval = (bool) 0;
-		att->attstorage = 'p';
 		att->attalign = 'i';
+		att->attstorage = 'p';
 		return false;
 	}
 
@@ -427,42 +425,63 @@ TupleDescInitEntry(TupleDesc desc,
 	typeForm = (Form_pg_type) GETSTRUCT(tuple);
 
 	att->atttypid = tuple->t_data->t_oid;
-	att->attalign = typeForm->typalign;
-
-	/* ------------------------
-	   If this attribute is a set, what is really stored in the
-	   attribute is the OID of a tuple in the pg_proc catalog.
-	   The pg_proc tuple contains the query string which defines
-	   this set - i.e., the query to run to get the set.
-	   So the atttypid (just assigned above) refers to the type returned
-	   by this query, but the actual length of this attribute is the
-	   length (size) of an OID.
-
-	   Why not just make the atttypid point to the OID type, instead
-	   of the type the query returns?  Because the executor uses the atttypid
-	   to tell the front end what type will be returned (in BeginCommand),
-	   and in the end the type returned will be the result of the query, not
-	   an OID.
-
-	   Why not wait until the return type of the set is known (i.e., the
-	   recursive call to the executor to execute the set has returned)
-	   before telling the front end what the return type will be?  Because
-	   the executor is a delicate thing, and making sure that the correct
-	   order of front-end commands is maintained is messy, especially
-	   considering that target lists may change as inherited attributes
-	   are considered, etc.  Ugh.
-	   -----------------------------------------
-	   */
+
+	/*------------------------
+	 * There are a couple of cases where we must override the information
+	 * stored in pg_type.
+	 *
+	 * First: if this attribute is a set, what is really stored in the
+	 * attribute is the OID of a tuple in the pg_proc catalog.
+	 * The pg_proc tuple contains the query string which defines
+	 * this set - i.e., the query to run to get the set.
+	 * So the atttypid (just assigned above) refers to the type returned
+	 * by this query, but the actual length of this attribute is the
+	 * length (size) of an OID.
+	 *
+	 * (Why not just make the atttypid point to the OID type, instead
+	 * of the type the query returns?  Because the executor uses the atttypid
+	 * to tell the front end what type will be returned (in BeginCommand),
+	 * and in the end the type returned will be the result of the query, not
+	 * an OID.)
+	 *
+	 * (Why not wait until the return type of the set is known (i.e., the
+	 * recursive call to the executor to execute the set has returned)
+	 * before telling the front end what the return type will be?  Because
+	 * the executor is a delicate thing, and making sure that the correct
+	 * order of front-end commands is maintained is messy, especially
+	 * considering that target lists may change as inherited attributes
+	 * are considered, etc.  Ugh.)
+	 *
+	 * Second: if we are dealing with a complex type (a tuple type), then
+	 * pg_type will say that the representation is the same as Oid.  But
+	 * if typmod is sizeof(Pointer) then the internal representation is
+	 * actually a pointer to a TupleTableSlot, and we have to substitute
+	 * that information.
+	 *
+	 * A set of complex type is first and foremost a set, so its
+	 * representation is Oid not pointer.  So, test that case first.
+	 *-----------------------------------------
+	 */
 	if (attisset)
 	{
 		att->attlen = sizeof(Oid);
 		att->attbyval = true;
+		att->attalign = 'i';
+		att->attstorage = 'p';
+	}
+	else if (typeForm->typtype == 'c' && typmod == sizeof(Pointer))
+	{
+		att->attlen = sizeof(Pointer);
+		att->attbyval = true;
+		att->attalign = 'd';	/* kluge to work with 8-byte pointers */
+		/* XXX ought to have a separate attalign value for pointers ... */
 		att->attstorage = 'p';
 	}
 	else
 	{
 		att->attlen = typeForm->typlen;
 		att->attbyval = typeForm->typbyval;
+		att->attalign = typeForm->typalign;
 		att->attstorage = typeForm->typstorage;
 	}
 
@@ -494,6 +513,7 @@ TupleDescMakeSelfReference(TupleDesc desc,
 	att->atttypid = TypeShellMake(relname);
 	att->attlen = sizeof(Oid);
 	att->attbyval = true;
+	att->attalign = 'i';
 	att->attstorage = 'p';
 	att->attnelems = 0;
 }
@@ -582,15 +602,13 @@ BuildDescForRelation(List *schema, char *relname)
 			 *	have a self reference, otherwise it's an error.
 			 * ----------------
 			 */
-			if (!strcmp(typename, relname))
+			if (strcmp(typename, relname) == 0)
 				TupleDescMakeSelfReference(desc, attnum, relname);
 			else
 				elog(ERROR, "DefineRelation: no such type %s",
 					 typename);
 		}
 
-		desc->attrs[attnum - 1]->atttypmod = entry->typename->typmod;
-
 		/* This is for constraints */
 		if (entry->is_not_null)
 			constr->has_not_null = true;