Date: Fri, 06 Sep 2024 16:50:30 +0900 (JST)
Message-Id: <20240906.165030.496293122169166261.ishii@postgresql.org>
To: ashutosh.bapat.oss@gmail.com
Cc: dgrowleyml@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: 
 <CAExHW5sWwtQaovcAfSvy5e9nh=XjhHpmcA==rijG5Jzh4atO_g@mail.gmail.com>
References: 
 <CAApHDvqgBcwV8iVZY0f1+D2TQrRAbjFA5AmkX-BHL2z9PhTQSQ@mail.gmail.com>
	<20240906.150237.57111972562260411.ishii@postgresql.org>
	<CAExHW5sWwtQaovcAfSvy5e9nh=XjhHpmcA==rijG5Jzh4atO_g@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/20240906.165030.496293122169166261.ishii%40postgresql.org>
Precedence: bulk

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

+1. Returning the storage type by the same function, not by a separate
function looks more natural.

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