mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Use tuplestore_clear instead of tuplestore_end in nodeTableFuncscan.c
Clearing the tuplestore rather than ending it avoids the need for various memory allocations, so is slightly more efficient. However, the main reason to do this is to correctly track the maximum storage used by the Table Function Scan node.40708acd65added additional EXPLAIN output for Table Function Scan nodes and that output does claim to be showing "Maximum Storage", which is not true, as the maximums could be lost after tuplestore_end() is called during a rescan. This causes misreporting when the final rescan of a Table Function Scan node uses less storage than some previous rescan. One example is: EXPLAIN ANALYZE SELECT count(*) FROM (VALUES ((SELECT jsonb_agg(g) FROM generate_series(1, 100000) g)), ('[1]'::jsonb)) v(j), LATERAL JSON_TABLE(v.j, '$[*]' COLUMNS (x int PATH '$')) jt; Without this fix, the reported Table Function Scan storage is for the '[1]'::jsonb case, whereas it should consider how much was used with the larger document and show the maximum of all rescans. TableFuncNext() used a NULL tupstore to tell that the rows still had to be fetched, so add a tupstore_filled flag for that, similar to what908a96861did for WindowAgg. Technically, this issue does exist in v18, but no backpatch as this is a fairly minor issue which was only noticed during a review of a similar feature being proposed for Function Scans. Author: Tatsuya Kawata <kawatatatsuya0913@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAHza6qdXjcqXv2aGU7OCakG0OmEsy9-jfP0Gpe2EL+k-AidOhw@mail.gmail.com
This commit is contained in:
@@ -59,11 +59,15 @@ TableFuncNext(TableFuncScanState *node)
|
||||
scanslot = node->ss.ss_ScanTupleSlot;
|
||||
|
||||
/*
|
||||
* If first time through, read all tuples from function and put them in a
|
||||
* tuplestore. Subsequent calls just fetch tuples from tuplestore.
|
||||
* If first time through (or first since a parameter change), read all
|
||||
* tuples from function and put them in a tuplestore. Subsequent calls
|
||||
* just fetch tuples from tuplestore.
|
||||
*/
|
||||
if (node->tupstore == NULL)
|
||||
if (!node->tupstore_filled)
|
||||
{
|
||||
tfuncFetchRows(node, node->ss.ps.ps_ExprContext);
|
||||
node->tupstore_filled = true;
|
||||
}
|
||||
|
||||
/*
|
||||
* Get the next tuple from tuplestore.
|
||||
@@ -172,6 +176,7 @@ ExecInitTableFuncScan(TableFuncScan *node, EState *estate, int eflags)
|
||||
"TableFunc per value context",
|
||||
ALLOCSET_DEFAULT_SIZES);
|
||||
scanstate->opaque = NULL; /* initialized at runtime */
|
||||
scanstate->tupstore_filled = false;
|
||||
|
||||
scanstate->ns_names = tf->ns_names;
|
||||
|
||||
@@ -244,18 +249,17 @@ ExecReScanTableFuncScan(TableFuncScanState *node)
|
||||
ExecScanReScan(&node->ss);
|
||||
|
||||
/*
|
||||
* Recompute when parameters are changed.
|
||||
* Recompute when parameters are changed. It's important to use
|
||||
* tuplestore_clear() rather than tuplestore_end() here so that we keep
|
||||
* track of the maximum storage used by all rescans.
|
||||
*/
|
||||
if (chgparam)
|
||||
{
|
||||
if (node->tupstore != NULL)
|
||||
{
|
||||
tuplestore_end(node->tupstore);
|
||||
node->tupstore = NULL;
|
||||
}
|
||||
tuplestore_clear(node->tupstore);
|
||||
node->tupstore_filled = false;
|
||||
}
|
||||
|
||||
if (node->tupstore != NULL)
|
||||
else if (node->tupstore != NULL)
|
||||
tuplestore_rescan(node->tupstore);
|
||||
}
|
||||
|
||||
@@ -275,9 +279,10 @@ tfuncFetchRows(TableFuncScanState *tstate, ExprContext *econtext)
|
||||
|
||||
Assert(tstate->opaque == NULL);
|
||||
|
||||
/* build tuplestore for the result */
|
||||
/* build tuplestore for the result, unless we have one from a prior scan */
|
||||
oldcxt = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
|
||||
tstate->tupstore = tuplestore_begin_heap(false, false, work_mem);
|
||||
if (tstate->tupstore == NULL)
|
||||
tstate->tupstore = tuplestore_begin_heap(false, false, work_mem);
|
||||
|
||||
/*
|
||||
* Each call to fetch a new set of rows - of which there may be very many
|
||||
|
||||
@@ -1999,6 +1999,7 @@ typedef struct TableFuncScanState
|
||||
int64 ordinal; /* row number to be output next */
|
||||
MemoryContext perTableCxt; /* per-table context */
|
||||
Tuplestorestate *tupstore; /* output tuple store */
|
||||
bool tupstore_filled; /* tuplestore filled yet? */
|
||||
} TableFuncScanState;
|
||||
|
||||
/* ----------------
|
||||
|
||||
Reference in New Issue
Block a user