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

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Tatsuo Ishii <ishii(at)postgresql(dot)org>
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-12 21:20:11
Message-ID: CAApHDvpF2_bn0WergLrxQvS8qHSKXV3-HOUjku49jCaK3DaMRQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 13 Sept 2024 at 00:12, Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
> In this patch I refactored show_material_info. I divided it into
> show_material_info and show_storage_info so that the latter can be
> used by other node types including window aggregate node. What do you
> think?

Yes, I think it's a good idea to move that into a helper function. If
you do the other node types, without that helper the could would have
to be repeated quite a few times. Maybe show_storage_info() can be
moved up with the other helper functions, say below
show_sortorder_options() ? It might be a good idea to keep the "if
(!es->analyze || tupstore == NULL)" checks in the calling function
rather than the helper too.

I thought about the location of the test for a while and read the
"This file is concerned with testing EXPLAIN in its own right."
comment at the top of that explain.out. I was trying to decide if
testing output of a specific node type met this or not. I can't pick
out any other tests there which are specific to a node type, so I'm
unsure if this is the location for it or not. However, to put it
anywhere else means having to add a plpgsql function to mask out the
unstable parts of EXPLAIN, so maybe the location is good as it saves
from having to do that. I'm 50/50 on this, so I'm happy to let you
decide. You could also shrink that 100 rows into a smaller number for
the generate_series without losing any coverage.

Aside from that, I think the patch is good. Thanks for working on it.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-09-12 21:33:46 Mutable foreign key constraints
Previous Message Andreas Karlsson 2024-09-12 21:09:10 Re: tiny step toward threading: reduce dependence on setlocale()