On Fri, 16 Oct 2026 at 04:01, Tatsuya Kawata
<[email protected]> wrote:
> Thanks for pointing that out! That makes sense. Attached is v5.
I looked at v5. Can we not switch back to how you had it in v2 by
moving the FunctionScanPerFuncState into execnodes.h and using that
directly in explain.c? It's the FunctionNext() changes that I'm not
keen on. I feel that's adding too much code to a function that
executes on every row. I quickly put that together in the attached,
which applies on top of v5. It seems to pass the tests. I didn't look
further than that, so please check.
David
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 1b7a3956e71..32b19d873c8 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3580,9 +3580,35 @@ show_ctescan_info(CteScanState *ctescanstate,
ExplainState *es)
static void
show_functionscan_info(FunctionScanState *fsstate, ExplainState *es)
{
+ bool totalDiskUsed = false;
+ int64 totalSpaceUsed = 0;
+
if (!es->analyze)
return;
+ /*
+ * Similar to the logic in ExecReScanFunctionScan(), we need to fetch
the
+ * stats from the final rescan (or the only one) and use these values if
+ * they're higher than what's already stored in FunctionScanState.stats.
+ */
+ for (int i = 0; i < fsstate->nfuncs; i++)
+ {
+ FunctionScanPerFuncState* fs = &fsstate->funcstates[i];
+
+ if (fs->tstore != NULL)
+ {
+ bool diskUsed;
+ int64 spaceUsed;
+
+ tuplestore_get_stats(fs->tstore, &diskUsed, &spaceUsed);
+ totalSpaceUsed += spaceUsed;
+ totalDiskUsed |= diskUsed;
+ }
+ }
+
+ fsstate->stats.maxSpaceUsed = Max(fsstate->stats.maxSpaceUsed,
totalSpaceUsed);
+ fsstate->stats.usedDisk |= totalDiskUsed;
+
/*
* Nothing to show for the leader if execution didn't get as far as
* creating any tuplestore.
diff --git a/src/backend/executor/nodeFunctionscan.c
b/src/backend/executor/nodeFunctionscan.c
index 8b37a812fd0..d6d1777d5ac 100644
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -29,27 +29,6 @@
#include "utils/memutils.h"
#include "utils/tuplestore.h"
-
-/*
- * Runtime data for each function being scanned.
- */
-typedef struct FunctionScanPerFuncState
-{
- SetExprState *setexpr; /* state of the expression being
evaluated */
- TupleDesc tupdesc; /* desc of the function result
type */
- int colcount; /* expected number of
result columns */
- Tuplestorestate *tstore; /* holds the function result set */
- int64 rowcount; /* # of rows in result set, -1
if not known */
- TupleTableSlot *func_slot; /* function result slot (or NULL) */
-
- /*
- * Storage statistics of the current tuplestore, to report in EXPLAIN
- * ANALYZE. Only meaningful while tstore is not NULL.
- */
- bool usedDisk;
- int64 spaceUsed;
-} FunctionScanPerFuncState;
-
static TupleTableSlot *FunctionNext(FunctionScanState *node);
static void update_storage_stats(FunctionScanState *node,
FunctionScanPerFuncState *fs);
@@ -113,10 +92,6 @@ FunctionNext(FunctionScanState *node)
* call is fast, so the overhead shouldn't be an issue.
*/
tuplestore_rescan(tstore);
-
- /* Collect its storage statistics for EXPLAIN ANALYZE */
- if (node->ss.ps.instrument)
- update_storage_stats(node,
&node->funcstates[0]);
}
/*
@@ -176,10 +151,6 @@ FunctionNext(FunctionScanState *node)
* call is fast, so the overhead shouldn't be an issue.
*/
tuplestore_rescan(fs->tstore);
-
- /* Collect its storage statistics for EXPLAIN ANALYZE */
- if (node->ss.ps.instrument)
- update_storage_stats(node, fs);
}
/*
@@ -375,8 +346,6 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate,
int eflags)
*/
fs->tstore = NULL;
fs->rowcount = -1;
- fs->usedDisk = false;
- fs->spaceUsed = 0;
/*
* Now build a tupdesc showing the result type we expect from
the
@@ -565,11 +534,31 @@ ExecEndFunctionScan(FunctionScanState *node)
if (node->shared_info != NULL && IsParallelWorker())
{
FunctionScanInstrumentation *si;
+ bool totalDiskUsed = false;
+ int64 totalSpaceUsed = 0;
+
+ for (int i = 0; i < node->nfuncs; i++)
+ {
+ FunctionScanPerFuncState* fs = &node->funcstates[i];
+
+ if (fs->tstore != NULL)
+ {
+ bool diskUsed;
+ int64 spaceUsed;
+
+ tuplestore_get_stats(fs->tstore, &diskUsed,
&spaceUsed);
+ totalSpaceUsed += spaceUsed;
+ totalDiskUsed |= diskUsed;
+ }
+ }
+
+ totalSpaceUsed = Max(node->stats.maxSpaceUsed, totalSpaceUsed);
+ totalDiskUsed |= node->stats.usedDisk;
Assert(ParallelWorkerNumber < node->shared_info->num_workers);
si = &node->shared_info->sinstrument[ParallelWorkerNumber];
- si->maxSpaceUsed = Max(si->maxSpaceUsed,
node->stats.maxSpaceUsed);
- si->usedDisk |= node->stats.usedDisk;
+ si->maxSpaceUsed = Max(si->maxSpaceUsed, totalSpaceUsed);
+ si->usedDisk |= totalDiskUsed;
}
/*
@@ -587,35 +576,6 @@ ExecEndFunctionScan(FunctionScanState *node)
}
}
-/*
- * update_storage_stats
- * Update the storage statistics to report in EXPLAIN ANALYZE with
the
- * function's tuplestore, which already holds the whole result set.
- *
- * A FunctionScan node has one tuplestore per function. We report the peak
- * of the total space used by the tuplestores existing at the same time. The
- * storage type is reported as disk if any tuplestore, of any function and in
- * any rescan, spilled to disk.
- */
-static void
-update_storage_stats(FunctionScanState *node, FunctionScanPerFuncState *fs)
-{
- int64 totalSpaceUsed = 0;
-
- tuplestore_get_stats(fs->tstore, &fs->usedDisk, &fs->spaceUsed);
-
- for (int i = 0; i < node->nfuncs; i++)
- {
- FunctionScanPerFuncState *f = &node->funcstates[i];
-
- if (f->tstore != NULL)
- totalSpaceUsed += f->spaceUsed;
- }
-
- node->stats.maxSpaceUsed = Max(node->stats.maxSpaceUsed,
totalSpaceUsed);
- node->stats.usedDisk |= fs->usedDisk;
-}
-
/* ----------------------------------------------------------------
* ExecReScanFunctionScan
*
@@ -652,18 +612,35 @@ ExecReScanFunctionScan(FunctionScanState *node)
if (chgparam)
{
ListCell *lc;
+ bool totalDiskUsed = false;
+ int64 totalSpaceUsed = 0;
i = 0;
foreach(lc, scan->functions)
{
RangeTblFunction *rtfunc = (RangeTblFunction *)
lfirst(lc);
+ FunctionScanPerFuncState *fs = &node->funcstates[i];
if (bms_overlap(chgparam, rtfunc->funcparams))
{
- if (node->funcstates[i].tstore != NULL)
+ if (fs->tstore != NULL)
{
+ /* Collect its storage statistics for
EXPLAIN ANALYZE */
+ if (node->ss.ps.instrument)
+ {
+ bool diskUsed;
+ int64 spaceUsed;
+
+
tuplestore_get_stats(fs->tstore, &diskUsed, &spaceUsed);
+ totalDiskUsed |= diskUsed;
+ totalSpaceUsed += spaceUsed;
+ }
+
tuplestore_end(node->funcstates[i].tstore);
node->funcstates[i].tstore = NULL;
+
+ node->stats.maxSpaceUsed =
Max(node->stats.maxSpaceUsed, totalSpaceUsed);
+ node->stats.usedDisk |= totalDiskUsed;
}
node->funcstates[i].rowcount = -1;
}
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4017d8b9c0f..4823a36349c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1910,6 +1910,20 @@ typedef struct SubqueryScanState
PlanState *subplan;
} SubqueryScanState;
+/* ----------------
+~* Runtime data for each function being scanned by a FunctionScan
node.
+ * ----------------
+ */
+typedef struct FunctionScanPerFuncState
+{
+ SetExprState *setexpr; /* state of the expression being
evaluated */
+ TupleDesc tupdesc; /* desc of the function result
type */
+ int colcount; /* expected number of
result columns */
+ Tuplestorestate *tstore; /* holds the function result set */
+ int64 rowcount; /* # of rows in result set, -1
if not known */
+ TupleTableSlot *func_slot; /* function result slot (or NULL) */
+} FunctionScanPerFuncState;
+
/* ----------------
* FunctionScanState information
*
@@ -1921,8 +1935,7 @@ typedef struct SubqueryScanState
* simple true if we have 1 function and
no ordinality
* ordinal current ordinal column value
* nfuncs number of functions being
executed
- * funcstates per-function execution states
(private in
- * nodeFunctionscan.c)
+ * funcstates per-function execution states
* argcontext memory context to evaluate
function arguments in
* stats storage statistics to report in
EXPLAIN ANALYZE
* shared_info statistics for parallel workers
diff --git a/src/test/regress/expected/explain.out
b/src/test/regress/expected/explain.out
index 1603c806876..3d6b8d12bde 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -858,7 +858,7 @@ select explain_filter('explain (analyze,buffers off,costs
off) select count(*) f
-- Test tuplestore storage usage in Function Scan with ROWS FROM, which uses
-- one tuplestore per function
-select explain_filter('explain (analyze,buffers off,costs off) select count(*)
from rows from (generate_series(1,2500), generate_series(1,2500)) a(n,m)');
+select explain_filter('explain (analyze,buffers off,costs off) select count(*)
from rows from (generate_series(1,1250), generate_series(1,1250)) a(n,m)');
explain_filter
------------------------------------------------------------------
Aggregate (actual time=N.N..N.N rows=N.N loops=N)
diff --git a/src/test/regress/expected/planner_est.out
b/src/test/regress/expected/planner_est.out
index fbe2d5ff1a1..6a8d223831d 100644
--- a/src/test/regress/expected/planner_est.out
+++ b/src/test/regress/expected/planner_est.out
@@ -29,8 +29,7 @@ BEGIN
EXECUTE format('explain (analyze %s, costs on, summary off, timing
off, buffers off) %s',
analyze_str, query)
LOOP
- -- Ignore Storage output because the size varies depending on
- -- the platform
+ -- ignore Storage output because the size varies depending on platform
CONTINUE WHEN ln ~ '^\s*Storage: ';
IF hide_costs = true THEN
diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql
index cb18f495343..aa58a548b6e 100644
--- a/src/test/regress/sql/explain.sql
+++ b/src/test/regress/sql/explain.sql
@@ -197,5 +197,5 @@ select explain_filter('explain (analyze,buffers off,costs
off) select count(*) f
select explain_filter('explain (analyze,buffers off,costs off) select count(*)
from generate_series(1,2500) a(n)');
-- Test tuplestore storage usage in Function Scan with ROWS FROM, which uses
-- one tuplestore per function
-select explain_filter('explain (analyze,buffers off,costs off) select count(*)
from rows from (generate_series(1,2500), generate_series(1,2500)) a(n,m)');
+select explain_filter('explain (analyze,buffers off,costs off) select count(*)
from rows from (generate_series(1,1250), generate_series(1,1250)) a(n,m)');
reset work_mem;
diff --git a/src/test/regress/sql/planner_est.sql
b/src/test/regress/sql/planner_est.sql
index 72aae8807ec..1da208eccce 100644
--- a/src/test/regress/sql/planner_est.sql
+++ b/src/test/regress/sql/planner_est.sql
@@ -30,8 +30,7 @@ BEGIN
EXECUTE format('explain (analyze %s, costs on, summary off, timing
off, buffers off) %s',
analyze_str, query)
LOOP
- -- Ignore Storage output because the size varies depending on
- -- the platform
+ -- ignore Storage output because the size varies depending on platform
CONTINUE WHEN ln ~ '^\s*Storage: ';
IF hide_costs = true THEN
