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

Now that commit fec0778c8 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
index e39f99fbb325f969ca2ce49ace348d4869571726..9e37fc3a14e25d661b162738fb8d9c244a923800 100644 (file)
--- 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
index 8eedb613a18397432b6f3fe024e2007e6bf1735d..74f8d89bcb1c25e83aee5166027124189c2a3c10 100644 (file)
--- 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
index af4f8c875c759079690f41dd7ef983d29a523da6..ad69a5ce43360526741b61203e49937faf15f6aa 100644 (file)
--- 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;