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, orlovmg(at)gmail(dot)com, jian(dot)universality(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
Date: 2024-09-23 06:28:32
Message-ID: 20240923.152832.587968014191090400.ishii@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> I looked at this and thought that one thing you might want to consider
> is adjusting show_storage_info() to accept the size and type
> parameters so you don't have to duplicate the formatting code in
> show_recursive_union_info().

I agree and made necessary changes. See attached v4 patches.

> The first of the new tests also isn't testing what you want it to
> test. Maybe you could add a "materialized" in there to stop the CTE
> being inlined:
>
> explain (analyze,costs off) with w(n) as materialized (select n from
> generate_series(1,10) a(n)) select sum(n) from w
>
> Also, I'm on the fence about if the new tests are worthwhile. I won't
> object to them, however. I just wanted to note that most of the
> complexity is in tuplestore.c of which there's already coverage for.
> The test's value is reduced by the fact that most of the interesting
> details have to be masked out due to possible platform variations in
> the number of bytes. Really the new tests are only testing that we
> display the storage details and maybe that the storage type came out
> as expected. It seems unlikely these would get broken. I'd say it's
> committers preference, however. I just wanted to add my thoughts. You
> have to offset the value against the fact that the expected output is
> likely to change over the years which adds to the burden of making
> changes to the EXPLAIN output.

After thinking more, I lean toward to your opinion. The new tests do
not give big value, but on the other hand they could become a burden
over the years. I do not include the new tests in the v4 patches.

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

Attachment Content-Type Size
v4-0001-Add-memory-disk-usage-for-more-executor-nodes.patch text/x-patch 6.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tender Wang 2024-09-23 06:40:00 Re: not null constraints, again
Previous Message David Rowley 2024-09-23 04:46:21 Re: ANALYZE ONLY