Date: Fri, 13 Sep 2024 18:31:53 +0900 (JST)
Message-Id: <20240913.183153.727679591062671857.ishii@postgresql.org>
To: ashutosh.bapat.oss@gmail.com
Cc: dgrowleyml@gmail.com, orlovmg@gmail.com, jian.universality@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: 
 <CAExHW5vRPRLvsZYLmNGcDLkPDWDHXGSWYjox-to-OsCVFETd3w@mail.gmail.com>
References: 
 <CAApHDvpF2_bn0WergLrxQvS8qHSKXV3-HOUjku49jCaK3DaMRQ@mail.gmail.com>
	<20240913.151133.589948606714055793.ishii@postgresql.org>
	<CAExHW5vRPRLvsZYLmNGcDLkPDWDHXGSWYjox-to-OsCVFETd3w@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/20240913.183153.727679591062671857.ishii%40postgresql.org>
Precedence: bulk

> The patch looks fine but it doesn't add a test case where Storage is
> Disk

We can test the case by setting work_mem to the minimum size (64kB)
and giving slightly larger "stop" parameter to generate_series.

> or the case when the last usage fit in memory but an earlier
> usage spilled to disk.

In my understanding once tuplestore changes the storage type to disk,
it never returns to the memory storage type in terms of
tuplestore_get_stats.  i.e. once state->usedDisk is set to true, it
never goes back to false. So the test case is not necessary.
David, am I correct?

> Do we want to cover those. This test would be
> the only one where those code paths could be tested.

I am fine to add the first test case.

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