From bc8393cf27731055467a83068c680c86f9c112ea Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 18 Jul 2019 10:37:13 -0400 Subject: [PATCH] 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 --- doc/src/sgml/spi.sgml | 20 ++++++++++++-------- src/backend/executor/spi.c | 22 +++++++++++----------- src/include/executor/spi.h | 4 ++-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml index e39f99fbb3..9e37fc3a14 100644 --- 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; - vals and tupdesc can - be used by SPI callers, the remaining fields are internal. - vals is an array of pointers to rows. (The number - of valid entries is given by SPI_processed.) + tupdesc, + vals, and + numvals + can be used by SPI callers; the remaining fields are internal. + vals is an array of pointers to rows. + The number of rows is given by numvals + (for somewhat historical reasons, this count is also returned + in SPI_processed). tupdesc is a row descriptor which you can pass to SPI functions dealing with rows. @@ -4631,12 +4635,12 @@ execq(PG_FUNCTION_ARGS) */ if (ret > 0 && SPI_tuptable != NULL) { - TupleDesc tupdesc = SPI_tuptable->tupdesc; SPITupleTable *tuptable = SPI_tuptable; + TupleDesc tupdesc = tuptable->tupdesc; char buf[8192]; uint64 j; - for (j = 0; j < proc; j++) + for (j = 0; j < tuptable->numvals; j++) { HeapTuple tuple = tuptable->vals[j]; int i; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 8eedb613a1..74f8d89bcb 100644 --- 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 af4f8c875c..ad69a5ce43 100644 --- 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; -- 2.40.0