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
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() |