Hi,

heap_deform_tuple and slot_deform_tuple contain duplicated code.  This
patch refactors them so that the guts are in a single place.

I have checked the resulting assembly code for heap_deform_tuple, and
with the "inline" declaration, the gcc version I have (4.7.2) generates
almost identical output both after the patch than before, thus there
shouldn't be any slowdown.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index e39b977..4afd998 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -865,6 +865,109 @@ heap_modifytuple(HeapTuple tuple,
 	return result;
 }
 
+
+/*
+ * heap_deconstruct_tuple
+ *		Guts of attribute extraction from a heap tuple.  See heap_deform_tuple
+ *		and slot_deform_tuple for the user-friendly versions.
+ *
+ * Its arguments are:
+ *	tupdata		pointer to the tuple data area
+ *	t_bits		pointer to the tuple null bitmask
+ *	hasnulls	"has nulls" bit in tuple infomask
+ *	start_attnum attnum at which to start decoding
+ *	end_attnum	attnum at which to stop decoding
+ *	att			the tuple's TupleDesc Form_pg_attribute pointer
+ *	offset		offset (into tp) at which start_attnum starts.
+ *	slowly		whether the cache can be used; updated same as offset.
+ *	values		output values
+ *	isnull		output nulls
+ *
+ * offset and slowly are updated to be the values corresponding to the next
+ * attribute to be decoded; thus this function can be used to deconstruct a
+ * tuple incrementally.  For the first call, start_attnum and offset should
+ * both be zero; a subsequent call would pass a start_attnum equal to the
+ * previous end_attnum.
+ *
+ * If hasnulls is false, NULL can be passed for t_bits.
+ */
+static inline void
+heap_deconstruct_tuple(char *tupdata,
+					   bits8 *t_bits,
+					   bool hasnulls,
+					   int start_attnum,
+					   int end_attnum,
+					   Form_pg_attribute *att,
+					   Datum *values,
+					   bool *isnull,
+					   long *offset,
+					   bool *slowly)
+{
+	int		attnum;
+	bool	slow;
+	long	off;
+
+	off = *offset;
+	slow = *slowly;
+
+	for (attnum = start_attnum; attnum < end_attnum; attnum++)
+	{
+		Form_pg_attribute thisatt = att[attnum];
+
+		if (hasnulls && att_isnull(attnum, t_bits))
+		{
+			values[attnum] = (Datum) 0;
+			isnull[attnum] = true;
+			slow = true;		/* can't use attcacheoff anymore */
+			continue;
+		}
+
+		isnull[attnum] = false;
+
+		if (!slow && thisatt->attcacheoff >= 0)
+			off = thisatt->attcacheoff;
+		else if (thisatt->attlen == -1)
+		{
+			/*
+			 * We can only cache the offset for a varlena attribute if the
+			 * offset is already suitably aligned, so that there would be no
+			 * pad bytes in any case: then the offset will be valid for either
+			 * an aligned or unaligned value.
+			 */
+			if (!slow &&
+				off == att_align_nominal(off, thisatt->attalign))
+				thisatt->attcacheoff = off;
+			else
+			{
+				off = att_align_pointer(off, thisatt->attalign, -1,
+										tp + off);
+				slow = true;
+			}
+		}
+		else
+		{
+			/* not varlena, so safe to use att_align_nominal */
+			off = att_align_nominal(off, thisatt->attalign);
+
+			if (!slow)
+				thisatt->attcacheoff = off;
+		}
+
+		values[attnum] = fetchatt(thisatt, tp + off);
+
+		off = att_addlength_pointer(off, thisatt->attlen, tp + off);
+
+		if (thisatt->attlen <= 0)
+			slow = true;		/* can't use attcacheoff anymore */
+	}
+
+	*slowly = slow;
+	*offset = off;
+
+	return attnum;
+}
+
+
 /*
  * heap_deform_tuple
  *		Given a tuple, extract data into values/isnull arrays; this is
@@ -910,56 +1013,8 @@ heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
 
 	off = 0;
 
-	for (attnum = 0; attnum < natts; attnum++)
-	{
-		Form_pg_attribute thisatt = att[attnum];
-
-		if (hasnulls && att_isnull(attnum, bp))
-		{
-			values[attnum] = (Datum) 0;
-			isnull[attnum] = true;
-			slow = true;		/* can't use attcacheoff anymore */
-			continue;
-		}
-
-		isnull[attnum] = false;
-
-		if (!slow && thisatt->attcacheoff >= 0)
-			off = thisatt->attcacheoff;
-		else if (thisatt->attlen == -1)
-		{
-			/*
-			 * We can only cache the offset for a varlena attribute if the
-			 * offset is already suitably aligned, so that there would be no
-			 * pad bytes in any case: then the offset will be valid for either
-			 * an aligned or unaligned value.
-			 */
-			if (!slow &&
-				off == att_align_nominal(off, thisatt->attalign))
-				thisatt->attcacheoff = off;
-			else
-			{
-				off = att_align_pointer(off, thisatt->attalign, -1,
-										tp + off);
-				slow = true;
-			}
-		}
-		else
-		{
-			/* not varlena, so safe to use att_align_nominal */
-			off = att_align_nominal(off, thisatt->attalign);
-
-			if (!slow)
-				thisatt->attcacheoff = off;
-		}
-
-		values[attnum] = fetchatt(thisatt, tp + off);
-
-		off = att_addlength_pointer(off, thisatt->attlen, tp + off);
-
-		if (thisatt->attlen <= 0)
-			slow = true;		/* can't use attcacheoff anymore */
-	}
+	attnum = heap_deconstruct_tuple(tp, bp, hasnulls, 0, natts, att,
+									values, isnull, &off, &slow);
 
 	/*
 	 * If tuple doesn't have all the atts indicated by tupleDesc, read the
@@ -1059,56 +1114,8 @@ slot_deform_tuple(TupleTableSlot *slot, int natts)
 
 	tp = (char *) tup + tup->t_hoff;
 
-	for (; attnum < natts; attnum++)
-	{
-		Form_pg_attribute thisatt = att[attnum];
-
-		if (hasnulls && att_isnull(attnum, bp))
-		{
-			values[attnum] = (Datum) 0;
-			isnull[attnum] = true;
-			slow = true;		/* can't use attcacheoff anymore */
-			continue;
-		}
-
-		isnull[attnum] = false;
-
-		if (!slow && thisatt->attcacheoff >= 0)
-			off = thisatt->attcacheoff;
-		else if (thisatt->attlen == -1)
-		{
-			/*
-			 * We can only cache the offset for a varlena attribute if the
-			 * offset is already suitably aligned, so that there would be no
-			 * pad bytes in any case: then the offset will be valid for either
-			 * an aligned or unaligned value.
-			 */
-			if (!slow &&
-				off == att_align_nominal(off, thisatt->attalign))
-				thisatt->attcacheoff = off;
-			else
-			{
-				off = att_align_pointer(off, thisatt->attalign, -1,
-										tp + off);
-				slow = true;
-			}
-		}
-		else
-		{
-			/* not varlena, so safe to use att_align_nominal */
-			off = att_align_nominal(off, thisatt->attalign);
-
-			if (!slow)
-				thisatt->attcacheoff = off;
-		}
-
-		values[attnum] = fetchatt(thisatt, tp + off);
-
-		off = att_addlength_pointer(off, thisatt->attlen, tp + off);
-
-		if (thisatt->attlen <= 0)
-			slow = true;		/* can't use attcacheoff anymore */
-	}
+	attnum = heap_deconstruct_tuple(tp, bp, hasnulls, attnum, natts, att,
+									values, isnull, &off, &slow);
 
 	/*
 	 * Save state for next execution
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to