Date: Thu, 05 Sep 2024 15:10:24 +0900 (JST)
Message-Id: <20240905.151024.1730395991018096027.ishii@postgresql.org>
To: jian.universality@gmail.com
Cc: dgrowleyml@gmail.com, ashutosh.bapat.oss@gmail.com,
 pgsql-hackers@postgresql.org
Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: 
 <CACJufxGTjvFj7FOchNuZMvjNeQY_6SWMzFk4x8WwuO608__DrQ@mail.gmail.com>
References: <20240709.154409.1091516017393224028.ishii@postgresql.org>
	<20240710.183610.1211398886785110571.ishii@postgresql.org>
	<CACJufxGTjvFj7FOchNuZMvjNeQY_6SWMzFk4x8WwuO608__DrQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20240905.151024.1730395991018096027.ishii%40postgresql.org>
Precedence: bulk

Hi,

> 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".

Thank you for looking into my patch. Unfortunately I need to work on
other issue before adjusting the comments because the fields might go
away if I change the tuplestore infrastructure per David's suggestion:
https://www.postgresql.org/message-id/CAApHDvoY8cibGcicLV0fNh%3D9JVx9PANcWvhkdjBnDCc9Quqytg%40mail.gmail.com

After this I will rebase the patches. This commit requires changes.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=908a968612f9ed61911d8ca0a185b262b82f1269

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp