From 4db1d9b56b28ea81c542d2276c3b494a86eb75dc Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 4 Aug 2017 15:06:29 -0700
Subject: [PATCH 12/16] Centralize slot deforming logic a bit.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/access/common/heaptuple.c | 148 +++++++++++-----------------------
 1 file changed, 47 insertions(+), 101 deletions(-)

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 13ee528e26..f77ea477fb 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -1046,6 +1046,7 @@ slot_deform_tuple(TupleTableSlot *slot, int natts)
 	long		off;			/* offset in tuple data */
 	bits8	   *bp = tup->t_bits;	/* ptr to null bitmap in tuple */
 	bool		slow;			/* can we use/set attcacheoff? */
+	int			valnatts = natts;
 
 	/*
 	 * Check whether the first call for this tuple, and initialize or restore
@@ -1065,6 +1066,9 @@ slot_deform_tuple(TupleTableSlot *slot, int natts)
 		slow = slot->tts_slow;
 	}
 
+
+	natts = Min(natts, Min(HeapTupleHeaderGetNatts(tuple->t_data), slot->tts_tupleDescriptor->natts));
+
 	tp = (char *) tup + tup->t_hoff;
 
 	for (; attnum < natts; attnum++)
@@ -1118,10 +1122,16 @@ slot_deform_tuple(TupleTableSlot *slot, int natts)
 			slow = true;		/* can't use attcacheoff anymore */
 	}
 
+	for (; attnum < valnatts; attnum++)
+	{
+		values[attnum] = 0;
+		isnull[attnum] = 1;
+	}
+
 	/*
 	 * Save state for next execution
 	 */
-	slot->tts_nvalid = attnum;
+	slot->tts_nvalid = valnatts;
 	slot->tts_off = off;
 	slot->tts_slow = slow;
 }
@@ -1142,46 +1152,38 @@ Datum
 slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull)
 {
 	HeapTuple	tuple = slot->tts_tuple;
-	TupleDesc	tupleDesc = slot->tts_tupleDescriptor;
-	HeapTupleHeader tup;
+	TupleDesc	tupleDesc PG_USED_FOR_ASSERTS_ONLY = slot->tts_tupleDescriptor;
 
 	/*
 	 * system attributes are handled by heap_getsysattr
 	 */
-	if (attnum <= 0)
+	if (unlikely(attnum <= 0))
 	{
-		if (tuple == NULL)		/* internal error */
-			elog(ERROR, "cannot extract system attribute from virtual tuple");
-		if (tuple == &(slot->tts_minhdr))	/* internal error */
-			elog(ERROR, "cannot extract system attribute from minimal tuple");
+
+		/* cannot extract system attribute from virtual tuple */
+		Assert(tuple);
+		/* "cannot extract system attribute from minimal tuple */
+		Assert(tuple != &(slot->tts_minhdr));
 		return heap_getsysattr(tuple, attnum, tupleDesc, isnull);
 	}
 
 	/*
 	 * fast path if desired attribute already cached
 	 */
-	if (attnum <= slot->tts_nvalid)
+	if (likely(attnum <= slot->tts_nvalid))
 	{
 		*isnull = slot->tts_isnull[attnum - 1];
 		return slot->tts_values[attnum - 1];
 	}
 
 	/*
-	 * return NULL if attnum is out of range according to the tupdesc
+	 * While tuples might possibly be wider than the slot, they should never
+	 * be accessed. We used to return NULL if so, but that a) isn't free b)
+	 * seems more likely to hide bugs than anything.
 	 */
-	if (attnum > tupleDesc->natts)
-	{
-		*isnull = true;
-		return (Datum) 0;
-	}
-
-	/*
-	 * otherwise we had better have a physical tuple (tts_nvalid should equal
-	 * natts in all virtual-tuple cases)
-	 */
-	if (tuple == NULL)			/* internal error */
-		elog(ERROR, "cannot extract attribute from empty tuple slot");
+	Assert(attnum <= tupleDesc->natts);
 
+#ifdef NOT_ANYMORE
 	/*
 	 * return NULL if attnum is out of range according to the tuple
 	 *
@@ -1195,26 +1197,13 @@ slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull)
 		*isnull = true;
 		return (Datum) 0;
 	}
+#endif
 
 	/*
-	 * check if target attribute is null: no point in groveling through tuple
+	 * otherwise we had better have a physical tuple (tts_nvalid should equal
+	 * natts in all virtual-tuple cases)
 	 */
-	if (HeapTupleHasNulls(tuple) && att_isnull(attnum - 1, tup->t_bits))
-	{
-		*isnull = true;
-		return (Datum) 0;
-	}
-
-	/*
-	 * If the attribute's column has been dropped, we force a NULL result.
-	 * This case should not happen in normal use, but it could happen if we
-	 * are executing a plan cached before the column was dropped.
-	 */
-	if (TupleDescAttr(tupleDesc, attnum - 1)->attisdropped)
-	{
-		*isnull = true;
-		return (Datum) 0;
-	}
+	Assert(tuple != NULL);
 
 	/*
 	 * Extract the attribute, along with any preceding attributes.
@@ -1238,8 +1227,7 @@ void
 slot_getallattrs(TupleTableSlot *slot)
 {
 	int			tdesc_natts = slot->tts_tupleDescriptor->natts;
-	int			attnum;
-	HeapTuple	tuple;
+	HeapTuple	tuple PG_USED_FOR_ASSERTS_ONLY;
 
 	/* Quick out if we have 'em all already */
 	if (slot->tts_nvalid == tdesc_natts)
@@ -1250,27 +1238,10 @@ slot_getallattrs(TupleTableSlot *slot)
 	 * natts in all virtual-tuple cases)
 	 */
 	tuple = slot->tts_tuple;
-	if (tuple == NULL)			/* internal error */
-		elog(ERROR, "cannot extract attribute from empty tuple slot");
+	Assert(tuple != NULL);
 
-	/*
-	 * load up any slots available from physical tuple
-	 */
-	attnum = HeapTupleHeaderGetNatts(tuple->t_data);
-	attnum = Min(attnum, tdesc_natts);
-
-	slot_deform_tuple(slot, attnum);
-
-	/*
-	 * If tuple doesn't have all the atts indicated by tupleDesc, read the
-	 * rest as null
-	 */
-	for (; attnum < tdesc_natts; attnum++)
-	{
-		slot->tts_values[attnum] = (Datum) 0;
-		slot->tts_isnull[attnum] = true;
-	}
-	slot->tts_nvalid = tdesc_natts;
+	slot_deform_tuple(slot, tdesc_natts);
+	Assert(tdesc_natts <= slot->tts_nvalid);
 }
 
 /*
@@ -1281,43 +1252,22 @@ slot_getallattrs(TupleTableSlot *slot)
 void
 slot_getsomeattrs(TupleTableSlot *slot, int attnum)
 {
-	HeapTuple	tuple;
-	int			attno;
-
 	/* Quick out if we have 'em all already */
 	if (slot->tts_nvalid >= attnum)
 		return;
 
 	/* Check for caller error */
-	if (attnum <= 0 || attnum > slot->tts_tupleDescriptor->natts)
-		elog(ERROR, "invalid attribute number %d", attnum);
+	Assert(attnum > 0);
+	Assert(attnum <= slot->tts_tupleDescriptor->natts);
 
 	/*
 	 * otherwise we had better have a physical tuple (tts_nvalid should equal
 	 * natts in all virtual-tuple cases)
 	 */
-	tuple = slot->tts_tuple;
-	if (tuple == NULL)			/* internal error */
-		elog(ERROR, "cannot extract attribute from empty tuple slot");
+	Assert(slot->tts_tuple != NULL); /* internal error */
 
-	/*
-	 * load up any slots available from physical tuple
-	 */
-	attno = HeapTupleHeaderGetNatts(tuple->t_data);
-	attno = Min(attno, attnum);
-
-	slot_deform_tuple(slot, attno);
-
-	/*
-	 * If tuple doesn't have all the atts indicated by tupleDesc, read the
-	 * rest as null
-	 */
-	for (; attno < attnum; attno++)
-	{
-		slot->tts_values[attno] = (Datum) 0;
-		slot->tts_isnull[attno] = true;
-	}
-	slot->tts_nvalid = attnum;
+	slot_deform_tuple(slot, attnum);
+	Assert(attnum <= slot->tts_nvalid);
 }
 
 /*
@@ -1329,38 +1279,34 @@ bool
 slot_attisnull(TupleTableSlot *slot, int attnum)
 {
 	HeapTuple	tuple = slot->tts_tuple;
-	TupleDesc	tupleDesc = slot->tts_tupleDescriptor;
+	TupleDesc	tupleDesc PG_USED_FOR_ASSERTS_ONLY = slot->tts_tupleDescriptor;
 
 	/*
 	 * system attributes are handled by heap_attisnull
 	 */
-	if (attnum <= 0)
+	if (unlikely(attnum <= 0))
 	{
-		if (tuple == NULL)		/* internal error */
-			elog(ERROR, "cannot extract system attribute from virtual tuple");
-		if (tuple == &(slot->tts_minhdr))	/* internal error */
-			elog(ERROR, "cannot extract system attribute from minimal tuple");
+		/* cannot extract system attribute from virtual tuple */
+		Assert(tuple);
+		/* "cannot extract system attribute from minimal tuple */
+		Assert(tuple != &(slot->tts_minhdr));
 		return heap_attisnull(tuple, attnum);
 	}
 
 	/*
 	 * fast path if desired attribute already cached
 	 */
-	if (attnum <= slot->tts_nvalid)
+	if (likely(attnum <= slot->tts_nvalid))
 		return slot->tts_isnull[attnum - 1];
 
-	/*
-	 * return NULL if attnum is out of range according to the tupdesc
-	 */
-	if (attnum > tupleDesc->natts)
-		return true;
+	/* Check for caller error */
+	Assert(attnum <= tupleDesc->natts);
 
 	/*
 	 * otherwise we had better have a physical tuple (tts_nvalid should equal
 	 * natts in all virtual-tuple cases)
 	 */
-	if (tuple == NULL)			/* internal error */
-		elog(ERROR, "cannot extract attribute from empty tuple slot");
+	Assert(tuple != NULL);
 
 	/* and let the tuple tell it */
 	return heap_attisnull(tuple, attnum);
-- 
2.14.1.2.g4274c698f4.dirty

