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.

7 files changed, 165 insertions, 91 deletions

diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index db40344dc46..7618cc55ae5 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: heapam.h,v 1.59 2000/11/30 18:38:46 tgl Exp $
+ * $Id: heapam.h,v 1.60 2000/12/27 23:59:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -90,9 +90,15 @@ typedef HeapAccessStatisticsData *HeapAccessStatistics;
 /* ----------------
  *		fastgetattr
  *
- *		This gets called many times, so we macro the cacheable and NULL
- *		lookups, and call noncachegetattr() for the rest.
+ *		Fetch a user attribute's value as a Datum (might be either a
+ *		value, or a pointer into the data area of the tuple).
+ *
+ *		This must not be used when a system attribute might be requested.
+ *		Furthermore, the passed attnum MUST be valid.  Use heap_getattr()
+ *		instead, if in doubt.
  *
+ *		This gets called many times, so we macro the cacheable and NULL
+ *		lookups, and call nocachegetattr() for the rest.
  * ----------------
  */
 
@@ -109,7 +115,7 @@ extern Datum nocachegetattr(HeapTuple tup, int attnum,
 	(																\
 		(tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ?			\
 		(															\
-			(Datum) fetchatt(&((tupleDesc)->attrs[(attnum)-1]),		\
+			fetchatt((tupleDesc)->attrs[(attnum)-1],				\
 				(char *) (tup)->t_data + (tup)->t_data->t_hoff +	\
 					(tupleDesc)->attrs[(attnum)-1]->attcacheoff)	\
 		)															\
@@ -132,9 +138,8 @@ extern Datum nocachegetattr(HeapTuple tup, int attnum,
 
 #else /* defined(DISABLE_COMPLEX_MACRO) */
 
-extern Datum
-fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
-			bool *isnull);
+extern Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
+						 bool *isnull);
 
 #endif /* defined(DISABLE_COMPLEX_MACRO) */
 
@@ -142,60 +147,39 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
 /* ----------------
  *		heap_getattr
  *
- *		Find a particular field in a row represented as a heap tuple.
- *		We return a pointer into that heap tuple, which points to the
- *		first byte of the value of the field in question.
+ *		Extract an attribute of a heap tuple and return it as a Datum.
+ *		This works for either system or user attributes.  The given attnum
+ *		is properly range-checked.
  *
- *		If the field in question has a NULL value, we return a null
- *		pointer and return <*isnull> == true.  Otherwise, we return
- *		<*isnull> == false.
+ *		If the field in question has a NULL value, we return a zero Datum
+ *		and set *isnull == true.  Otherwise, we set *isnull == false.
  *
  *		<tup> is the pointer to the heap tuple.  <attnum> is the attribute
  *		number of the column (field) caller wants.	<tupleDesc> is a
  *		pointer to the structure describing the row and all its fields.
- *
- *		Because this macro is often called with constants, it generates
- *		compiler warnings about 'left-hand comma expression has no effect.
- *
  * ----------------
  */
 #define heap_getattr(tup, attnum, tupleDesc, isnull) \
 ( \
-	AssertMacro((tup) != NULL && \
-		(attnum) > FirstLowInvalidHeapAttributeNumber && \
-		(attnum) != 0), \
-	((attnum) > (int) (tup)->t_data->t_natts) ? \
-	( \
-		((isnull) ? (*(isnull) = true) : (dummyret)NULL), \
-		(Datum)NULL \
-	) \
-	: \
+	AssertMacro((tup) != NULL), \
 	( \
 		((attnum) > 0) ? \
 		( \
-			fastgetattr((tup), (attnum), (tupleDesc), (isnull)) \
-		) \
-		: \
-		( \
-			((isnull) ? (*(isnull) = false) : (dummyret)NULL), \
-			((attnum) == SelfItemPointerAttributeNumber) ? \
+			((attnum) > (int) (tup)->t_data->t_natts) ? \
 			( \
-				(Datum)((char *)&((tup)->t_self)) \
+				((isnull) ? (*(isnull) = true) : (dummyret)NULL), \
+				(Datum)NULL \
 			) \
 			: \
-			(((attnum) == TableOidAttributeNumber) ? \
-			( \
-				(Datum)((tup)->t_tableOid) \
-			) \
-            : \
-			( \
-				(Datum)*(unsigned int *) \
-					((char *)(tup)->t_data + heap_sysoffset[-(attnum)-1]) \
-			)) \
+				fastgetattr((tup), (attnum), (tupleDesc), (isnull)) \
 		) \
+		: \
+			heap_getsysattr((tup), (attnum), (isnull)) \
 	) \
 )
 
+extern Datum heap_getsysattr(HeapTuple tup, int attnum, bool *isnull);
+
 extern HeapAccessStatistics heap_access_stats;	/* in stats.c */
 
 /* ----------------
@@ -238,8 +222,6 @@ extern void DataFill(char *data, TupleDesc tupleDesc,
 		 Datum *value, char *nulls, uint16 *infomask,
 		 bits8 *bit);
 extern int	heap_attisnull(HeapTuple tup, int attnum);
-extern int	heap_sysattrlen(AttrNumber attno);
-extern bool heap_sysattrbyval(AttrNumber attno);
 extern Datum nocachegetattr(HeapTuple tup, int attnum,
 			   TupleDesc att, bool *isnull);
 extern HeapTuple heap_copytuple(HeapTuple tuple);
diff --git a/src/include/access/htup.h b/src/include/access/htup.h
index eebc1570f54..56631043e3b 100644
--- a/src/include/access/htup.h
+++ b/src/include/access/htup.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: htup.h,v 1.41 2000/11/30 08:46:25 vadim Exp $
+ * $Id: htup.h,v 1.42 2000/12/27 23:59:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -147,6 +147,9 @@ typedef struct xl_heap_update
 #define MaxAttrSize		(10 * 1024 * 1024)
 
 
+/*
+ * Attribute numbers for the system-defined attributes
+ */
 #define SelfItemPointerAttributeNumber			(-1)
 #define ObjectIdAttributeNumber					(-2)
 #define MinTransactionIdAttributeNumber			(-3)
@@ -156,9 +159,6 @@ typedef struct xl_heap_update
 #define TableOidAttributeNumber			        (-7)
 #define FirstLowInvalidHeapAttributeNumber		(-8)
 
-/* If you make any changes above, the order of offsets in this must change */
-extern long heap_sysoffset[];
-
 /*
  * This new HeapTuple for version >= 6.5 and this is why it was changed:
  *
diff --git a/src/include/access/itup.h b/src/include/access/itup.h
index 18639eb7f20..f840ead4cb7 100644
--- a/src/include/access/itup.h
+++ b/src/include/access/itup.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: itup.h,v 1.26 2000/11/30 18:38:46 tgl Exp $
+ * $Id: itup.h,v 1.27 2000/12/27 23:59:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -111,7 +111,7 @@ typedef RetrieveIndexResultData *RetrieveIndexResult;
 	( \
 		(tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
 		( \
-			(Datum) fetchatt(&((tupleDesc)->attrs[(attnum)-1]), \
+			fetchatt((tupleDesc)->attrs[(attnum)-1], \
 			(char *) (tup) + \
 			( \
 				IndexTupleHasMinHeader(tup) ? \
diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h
index ab856087a70..eb977189e14 100644
--- a/src/include/access/tupmacs.h
+++ b/src/include/access/tupmacs.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: tupmacs.h,v 1.14 2000/03/17 02:36:37 tgl Exp $
+ * $Id: tupmacs.h,v 1.15 2000/12/27 23:59:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -22,50 +22,79 @@
 #define att_isnull(ATT, BITS) (!((BITS)[(ATT) >> 3] & (1 << ((ATT) & 0x07))))
 
 /*
- * given a Form_pg_attribute and a pointer into a tuple's data
- * area, return the correct value or pointer.
+ * Given a Form_pg_attribute and a pointer into a tuple's data area,
+ * return the correct value or pointer.
  *
- * We return a 4 byte (char *) value in all cases.	If the attribute has
- * "byval" false or has variable length, we return the same pointer
- * into the tuple data area that we're passed.  Otherwise, we return
- * the 1, 2, or 4 bytes pointed to by it, properly extended to 4
- * bytes, depending on the length of the attribute.
+ * We return a Datum value in all cases.  If the attribute has "byval" false,
+ * we return the same pointer into the tuple data area that we're passed.
+ * Otherwise, we return the correct number of bytes fetched from the data
+ * area and extended to Datum form.
  *
- * note that T must already be properly LONGALIGN/SHORTALIGN'd for
- * this to work correctly.
+ * On machines where Datum is 8 bytes, we support fetching 8-byte byval
+ * attributes; otherwise, only 1, 2, and 4-byte values are supported.
  *
- * the double-cast is to stop gcc from (correctly) complaining about
- * casting integer types with size < sizeof(char *) to (char *).
- * sign-extension may get weird if you use an integer type that
- * isn't the same size as (char *) for the first cast.  (on the other
- * hand, it's safe to use another type for the (foo *)(T).)
- *
- * attbyval seems to be fairly redundant.  We have to return a pointer if
- * the value is longer than 4 bytes or has variable length; returning the
- * value would be useless.	In fact, for at least the variable length case,
- * the caller assumes we return a pointer regardless of attbyval.
- * I would eliminate attbyval altogether, but I don't know how.  -BRYANH.
+ * Note that T must already be properly aligned for this to work correctly.
+ */
+#define fetchatt(A,T) fetch_att(T, (A)->attbyval, (A)->attlen)
+
+/*
+ * Same, but work from byval/len parameters rather than Form_pg_attribute.
  */
-#define fetchatt(A, T) \
+#if SIZEOF_DATUM == 8
+
+#define fetch_att(T,attbyval,attlen) \
 ( \
-	(*(A))->attbyval && (*(A))->attlen != -1 ? \
+	(attbyval) ? \
 	( \
-		(*(A))->attlen > (int) sizeof(int16) ? \
+		(attlen) == (int) sizeof(Datum) ? \
+			*((Datum *)(T)) \
+		: \
+	  ( \
+		(attlen) == (int) sizeof(int32) ? \
+			Int32GetDatum(*((int32 *)(T))) \
+		: \
 		( \
-			(char *) (long) *((int32 *)(T)) \
+			(attlen) == (int) sizeof(int16) ? \
+				Int16GetDatum(*((int16 *)(T))) \
+			: \
+			( \
+				AssertMacro((attlen) == 1), \
+				CharGetDatum(*((char *)(T))) \
+			) \
 		) \
+	  ) \
+	) \
+	: \
+	PointerGetDatum((char *) (T)) \
+)
+
+#else /* SIZEOF_DATUM != 8 */
+
+#define fetch_att(T,attbyval,attlen) \
+( \
+	(attbyval) ? \
+	( \
+		(attlen) == (int) sizeof(int32) ? \
+			Int32GetDatum(*((int32 *)(T))) \
 		: \
 		( \
-			(*(A))->attlen < (int) sizeof(int16) ? \
-				(char *) (long) *((char *)(T)) \
+			(attlen) == (int) sizeof(int16) ? \
+				Int16GetDatum(*((int16 *)(T))) \
 			: \
-				(char *) (long) *((int16 *)(T))) \
+			( \
+				AssertMacro((attlen) == 1), \
+				CharGetDatum(*((char *)(T))) \
+			) \
 		) \
+	) \
 	: \
-	(char *) (T) \
+	PointerGetDatum((char *) (T)) \
 )
 
-/* att_align aligns the given offset as needed for a datum of length attlen
+#endif /* SIZEOF_DATUM == 8 */
+
+/*
+ * att_align aligns the given offset as needed for a datum of length attlen
  * and alignment requirement attalign.	In practice we don't need the length.
  * The attalign cases are tested in what is hopefully something like their
  * frequency of occurrence.
@@ -81,6 +110,10 @@
 		))) \
 )
 
+/*
+ * att_addlength increments the given offset by the length of the attribute.
+ * attval is only accessed if we are dealing with a varlena attribute.
+ */
 #define att_addlength(cur_offset, attlen, attval) \
 ( \
 	((attlen) != -1) ? \
@@ -93,4 +126,60 @@
 	) \
 )
 
+/*
+ * store_att_byval is a partial inverse of fetch_att: store a given Datum
+ * value into a tuple data area at the specified address.  However, it only
+ * handles the byval case, because in typical usage the caller needs to
+ * distinguish by-val and by-ref cases anyway, and so a do-it-all macro
+ * wouldn't be convenient.
+ */
+#if SIZEOF_DATUM == 8
+
+#define store_att_byval(T,newdatum,attlen) \
+	do { \
+		switch (attlen) \
+		{ \
+			case sizeof(char): \
+				*(char *) (T) = DatumGetChar(newdatum); \
+				break; \
+			case sizeof(int16): \
+				*(int16 *) (T) = DatumGetInt16(newdatum); \
+				break; \
+			case sizeof(int32): \
+				*(int32 *) (T) = DatumGetInt32(newdatum); \
+				break; \
+			case sizeof(Datum): \
+				*(Datum *) (T) = (newdatum); \
+				break; \
+			default: \
+				elog(ERROR, "store_att_byval: unsupported byval length %d", \
+					 (int) (attlen)); \
+				break; \
+		} \
+	} while (0)
+
+#else /* SIZEOF_DATUM != 8 */
+
+#define store_att_byval(T,newdatum,attlen) \
+	do { \
+		switch (attlen) \
+		{ \
+			case sizeof(char): \
+				*(char *) (T) = DatumGetChar(newdatum); \
+				break; \
+			case sizeof(int16): \
+				*(int16 *) (T) = DatumGetInt16(newdatum); \
+				break; \
+			case sizeof(int32): \
+				*(int32 *) (T) = DatumGetInt32(newdatum); \
+				break; \
+			default: \
+				elog(ERROR, "store_att_byval: unsupported byval length %d", \
+					 (int) (attlen)); \
+				break; \
+		} \
+	} while (0)
+
+#endif /* SIZEOF_DATUM == 8 */
+
 #endif
diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h
index b1d51d520de..4ebe9499ad8 100644
--- a/src/include/catalog/catalog.h
+++ b/src/include/catalog/catalog.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: catalog.h,v 1.13 2000/10/16 14:52:26 vadim Exp $
+ * $Id: catalog.h,v 1.14 2000/12/27 23:59:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -30,7 +30,7 @@ extern char *GetDatabasePath(Oid tblNode);
 
 extern bool IsSystemRelationName(const char *relname);
 extern bool IsSharedSystemRelationName(const char *relname);
+
 extern Oid	newoid(void);
-extern void fillatt(TupleDesc att);
 
 #endif	 /* CATALOG_H */
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index f9f7372fd78..d5e0c58dd19 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: pg_type.h,v 1.98 2000/11/30 18:38:47 tgl Exp $
+ * $Id: pg_type.h,v 1.99 2000/12/27 23:59:13 tgl Exp $
  *
  * NOTES
  *	  the genbki.sh script reads this file and generates .bki
@@ -51,13 +51,11 @@ CATALOG(pg_type) BOOTSTRAP
 
 	/*
 	 * typbyval determines whether internal Postgres routines pass a value
-	 * of this type by value or by reference.  Only char, short, and int-
-	 * equivalent items can be passed by value, so if the type is not 1,
-	 * 2, or 4 bytes long, Postgres does not have the option of passing by
-	 * value and so typbyval had better be FALSE.  Variable-length types
-	 * are always passed by reference. Note that typbyval can be false
-	 * even if the length would allow pass-by-value; this is currently
-	 * true for type float4, for example.
+	 * of this type by value or by reference.  typbyval had better be FALSE
+	 * if the length is not 1, 2, or 4 (or 8 on 8-byte-Datum machines).
+	 * Variable-length types are always passed by reference. Note that
+	 * typbyval can be false even if the length would allow pass-by-value;
+	 * this is currently true for type float4, for example.
 	 */
 	bool		typbyval;
 
diff --git a/src/include/config.h.in b/src/include/config.h.in
index 77a61e64106..8be472c3fec 100644
--- a/src/include/config.h.in
+++ b/src/include/config.h.in
@@ -8,7 +8,7 @@
  * or in config.h afterwards.  Of course, if you edit config.h, then your
  * changes will be overwritten the next time you run configure.
  *
- * $Id: config.h.in,v 1.153 2000/12/02 18:16:40 tgl Exp $
+ * $Id: config.h.in,v 1.154 2000/12/27 23:59:14 tgl Exp $
  */
 
 #ifndef CONFIG_H
@@ -582,6 +582,11 @@ extern void srandom(unsigned int seed);
 #undef INT64_FORMAT
 
 /*
+ * We need a #define symbol for sizeof(Datum) for use in some #if tests.
+ */
+#undef SIZEOF_DATUM
+
+/*
  * These must be defined as the alignment requirement (NOT the size) of
  * each of the basic C data types (except char, which we assume has align 1).
  * MAXIMUM_ALIGNOF is the largest alignment requirement for any C data type.