From a7f8cd811d65d1a51be87bdb890d7ebf7d49aa45 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 5 Oct 2026 15:58:13 +1300 Subject: [PATCH] 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. 40708acd65 added 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 what 908a96861 did 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 Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAHza6qdXjcqXv2aGU7OCakG0OmEsy9-jfP0Gpe2EL+k-AidOhw@mail.gmail.com --- src/backend/executor/nodeTableFuncscan.c | 29 ++++++++++++++---------- src/include/nodes/execnodes.h | 1 + 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/backend/executor/nodeTableFuncscan.c b/src/backend/executor/nodeTableFuncscan.c index 9394156f405..61310427944 100644 --- a/src/backend/executor/nodeTableFuncscan.c +++ b/src/backend/executor/nodeTableFuncscan.c @@ -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 diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 91bb0bd2e13..0998c9b2b9d 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -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; /* ----------------