Further adjust SPITupleTable to provide a public row-count field.

tglsfdc · tglsfdc · commit bc8393cf2773 · 2019-07-18T10:37:13.000-04:00
Now that commit fec0778 drew a clear line between public and private fields in SPITupleTable, it seems pretty silly that the count of valid tuples isn't on the public side of that line. The reason why not was that there wasn't such a count. For reasons lost in the mists of time, spi.c preferred to keep a count of remaining free entries in the array. But that seems pretty pointless: it's unlike the way we handle similar code everywhere else, and it involves extra subtractions that surely outweigh having to do a comparison rather than test-for-zero to check for array-full. Hence, rearrange so that this code does the expansible array logic the same as everywhere else, with a count of valid entries alongside the allocated array length. And document the count as public. I looked for core-code callers where it would make sense to start relying on tuptable->numvals rather than the separate SPI_processed variable. Right now there don't seem to be places where it'd be a win to do so without more code restructuring than I care to undertake today. In principle, though, having SPITupleTables be fully self-contained should be helpful down the line. Discussion: https://postgr.es/m/16852.1563395722@sss.pgh.pa.us
diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
@@ -323,19 +323,23 @@ typedef struct SPITupleTable
     /* Public members */
     TupleDesc   tupdesc;        /* tuple descriptor */
     HeapTuple  *vals;           /* array of tuples */
+    uint64      numvals;        /* number of valid tuples */
 
     /* Private members, not intended for external callers */
+    uint64      alloced;        /* allocated length of vals array */
     MemoryContext tuptabcxt;    /* memory context of result table */
-    uint64      alloced;        /* # of alloced vals */
-    uint64      free;           /* # of free vals */
     slist_node  next;           /* link for internal bookkeeping */
     SubTransactionId subid;     /* subxact in which tuptable was created */
 } SPITupleTable;
 </programlisting>
-   <structfield>vals</structfield> and <structfield>tupdesc</structfield> can
-   be used by SPI callers, the remaining fields are internal.
-   <structfield>vals</structfield> is an array of pointers to rows.  (The number
-   of valid entries is given by <varname>SPI_processed</varname>.)
+   <structfield>tupdesc</structfield>,
+   <structfield>vals</structfield>, and
+   <structfield>numvals</structfield>
+   can be used by SPI callers; the remaining fields are internal.
+   <structfield>vals</structfield> is an array of pointers to rows.
+   The number of rows is given by <structfield>numvals</structfield>
+   (for somewhat historical reasons, this count is also returned
+   in <varname>SPI_processed</varname>).
    <structfield>tupdesc</structfield> is a row descriptor which you can pass to
    SPI functions dealing with rows.
   </para>
@@ -4631,12 +4635,12 @@ execq(PG_FUNCTION_ARGS)
      */
     if (ret &gt; 0 &amp;&amp; SPI_tuptable != NULL)
     {
-        TupleDesc tupdesc = SPI_tuptable-&gt;tupdesc;
         SPITupleTable *tuptable = SPI_tuptable;
+        TupleDesc tupdesc = tuptable-&gt;tupdesc;
         char buf[8192];
         uint64 j;
 
-        for (j = 0; j &lt; proc; j++)
+        for (j = 0; j &lt; tuptable-&gt;numvals; j++)
         {
             HeapTuple tuple = tuptable-&gt;vals[j];
             int i;
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
@@ -1872,8 +1872,9 @@ spi_dest_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 	slist_push_head(&_SPI_current->tuptables, &tuptable->next);
 
 	/* set up initial allocations */
-	tuptable->alloced = tuptable->free = 128;
+	tuptable->alloced = 128;
 	tuptable->vals = (HeapTuple *) palloc(tuptable->alloced * sizeof(HeapTuple));
+	tuptable->numvals = 0;
 	tuptable->tupdesc = CreateTupleDescCopy(typeinfo);
 
 	MemoryContextSwitchTo(oldcxt);
@@ -1899,18 +1900,18 @@ spi_printtup(TupleTableSlot *slot, DestReceiver *self)
 
 	oldcxt = MemoryContextSwitchTo(tuptable->tuptabcxt);
 
-	if (tuptable->free == 0)
+	if (tuptable->numvals >= tuptable->alloced)
 	{
 		/* Double the size of the pointer array */
-		tuptable->free = tuptable->alloced;
-		tuptable->alloced += tuptable->free;
+		uint64		newalloced = tuptable->alloced * 2;
+
 		tuptable->vals = (HeapTuple *) repalloc_huge(tuptable->vals,
-													 tuptable->alloced * sizeof(HeapTuple));
+													 newalloced * sizeof(HeapTuple));
+		tuptable->alloced = newalloced;
 	}
 
-	tuptable->vals[tuptable->alloced - tuptable->free] =
-		ExecCopySlotHeapTuple(slot);
-	(tuptable->free)--;
+	tuptable->vals[tuptable->numvals] = ExecCopySlotHeapTuple(slot);
+	(tuptable->numvals)++;
 
 	MemoryContextSwitchTo(oldcxt);
 
@@ -2324,8 +2325,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
 
 				/* Update "processed" if stmt returned tuples */
 				if (_SPI_current->tuptable)
-					_SPI_current->processed = _SPI_current->tuptable->alloced -
-						_SPI_current->tuptable->free;
+					_SPI_current->processed = _SPI_current->tuptable->numvals;
 
 				res = SPI_OK_UTILITY;
 
@@ -2694,7 +2694,7 @@ _SPI_checktuples(void)
 
 	if (tuptable == NULL)		/* spi_dest_startup was not called */
 		failed = true;
-	else if (processed != (tuptable->alloced - tuptable->free))
+	else if (processed != tuptable->numvals)
 		failed = true;
 
 	return failed;
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
@@ -24,11 +24,11 @@ typedef struct SPITupleTable
 	/* Public members */
 	TupleDesc	tupdesc;		/* tuple descriptor */
 	HeapTuple  *vals;			/* array of tuples */
+	uint64		numvals;		/* number of valid tuples */
 
 	/* Private members, not intended for external callers */
+	uint64		alloced;		/* allocated length of vals array */
 	MemoryContext tuptabcxt;	/* memory context of result table */
-	uint64		alloced;		/* # of alloced vals */
-	uint64		free;			/* # of free vals */
 	slist_node	next;			/* link for internal bookkeeping */
 	SubTransactionId subid;		/* subxact in which tuptable was created */
 } SPITupleTable;
