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

From: Tatsuo Ishii <ishii(at)postgresql(dot)org>
To: dgrowleyml(at)gmail(dot)com
Cc: ashutosh(dot)bapat(dot)oss(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
Date: 2024-09-12 02:03:54
Message-ID: 20240912.110354.2047263122138484536.ishii@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Fri, 6 Sept 2024 at 19:08, Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>> The changes look better. A nitpick though. With their definitions
>> changed, I think it's better to change the names of the functions
>> since their purpose has changed. Right now they report the storage
>> type and size used, respectively, at the time of calling the function.
>> With this patch, they report maximum space ever used and the storage
>> corresponding to the maximum space. tuplestore_space_used() may be
>> changed to tuplestore_maxspace_used(). I am having difficulty with
>> tuplestore_storage_type_name(); tuplestore_largest_storage_type_name()
>> seems mouthful and yet not doing justice to the functionality. It
>> might be better to just have one funciton tuplestore_maxspace_used()
>> which returns both the maximum space used as well as the storage type
>> when maximum space was used.
>
> How about just removing tuplestore_storage_type_name() and
> tuplestore_space_used() and adding tuplestore_get_stats(). I did take
> some inspiration from tuplesort.c for this, so maybe we can defer back
> there for further guidance. I'm not so sure it's worth having a stats
> struct type like tuplesort.c has. All we need is a char ** and an
> int64 * output parameter to pass to the stats function. I don't think
> we need to copy the tuplesort_method_name(). It seems fine just to
> point the output parameter of the stats function at the statically
> allocated constant.

Are you going to push the changes to tuplestore.c anytime soon? I
would like to rebase my patch[1] but the patch could be affected by
the tuplestore API change.

Best reagards,

[1] https://www.postgresql.org/message-id/CAApHDvoY8cibGcicLV0fNh%3D9JVx9PANcWvhkdjBnDCc9Quqytg%40mail.gmail.com
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2024-09-12 02:07:37 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
Previous Message Tom Lane 2024-09-12 02:02:59 Re: Remove shadowed declaration warnings