Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN

On Wed, Jul 10, 2024 at 5:36 PM Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
>
>
> Attached are the v2 patches. As suggested by David, I split them
> into multiple patches so that each patch implements the feature for
> each node. You need to apply the patches in the order of patch number
> (if you want to apply all of them, "git apply v2-*.patch" should
> work).
>
> v2-0001-Refactor-show_material_info.patch:
> This refactors show_material_info(). The guts are moved to new
> show_storage_info() so that it can be shared by not only Materialized
> node.
>
> v2-0002-Add-memory-disk-usage-for-CTE-Scan-nodes-in-EXPLA.patch:
> This adds memory/disk usage for CTE Scan nodes in EXPLAIN (ANALYZE) command.
>
> v2-0003-Add-memory-disk-usage-for-Table-Function-Scan-nod.patch:
> This adds memory/disk usage for Table Function Scan nodes in EXPLAIN (ANALYZE) command.
>
> v2-0004-Add-memory-disk-usage-for-Recursive-Union-nodes-i.patch:
> This adds memory/disk usage for Recursive Union nodes in EXPLAIN
> (ANALYZE) command. Also show_storage_info() is changed so that it
> accepts int64 storage_used, char *storage_type arguments. They are
> used if the target node uses multiple tuplestores, in case a simple
> call to tuplestore_space_used() does not work. Such executor nodes
> need to collect storage_used while running the node. This type of node
> includes Recursive Union and Window Aggregate.
>
> v2-0005-Add-memory-disk-usage-for-Window-Aggregate-nodes-.patch: This
> adds memory/disk usage for Window Aggregate nodes in EXPLAIN (ANALYZE)
> command. Note that if David's proposal
> https://www.postgresql.org/message-id/CAHoyFK9n-QCXKTUWT_xxtXninSMEv%2BgbJN66-y6prM3f4WkEHw%40mail.gmail.com
> is committed, this will need to be adjusted.
>
> For a demonstration, how storage/memory usage is shown in EXPLAIN
> (notice "Storage: Memory Maximum Storage: 120kB" etc.). The script
> used is attached (test.sql.txt). The SQLs are shamelessly copied from
> David's example and the regression test (some of them were modified by
> me).
>

hi. I can roughly understand it.

I have one minor issue with the comment.

typedef struct RecursiveUnionState
{
PlanState ps; /* its first field is NodeTag */
bool recursing;
bool intermediate_empty;
Tuplestorestate *working_table;
Tuplestorestate *intermediate_table;
int64 storageSize; /* max storage size Tuplestore */
char *storageType; /* the storage type above */
....
}

"/* the storage type above */"
is kind of ambiguous, since there is more than one Tuplestorestate.

i think it roughly means: the storage type of working_table
while the max storage of working_table.

typedef struct WindowAggState
{
ScanState ss; /* its first field is NodeTag */

/* these fields are filled in by ExecInitExpr: */
List *funcs; /* all WindowFunc nodes in targetlist */
int numfuncs; /* total number of window functions */
int numaggs; /* number that are plain aggregates */

WindowStatePerFunc perfunc; /* per-window-function information */
WindowStatePerAgg peragg; /* per-plain-aggregate information */
ExprState *partEqfunction; /* equality funcs for partition columns */
ExprState *ordEqfunction; /* equality funcs for ordering columns */
Tuplestorestate *buffer; /* stores rows of current partition */
int64 storageSize; /* max storage size in buffer */
char *storageType; /* the storage type above */
}

" /* the storage type above */"
I think it roughly means:
" the storage type of WindowAggState->buffer while the max storage of
WindowAggState->buffer".