Re: Summary Sort workers Stats in EXPLAIN ANALYZE

I have updated the patch addressing the review comments, but I didn't moved this code block into VERBOSE mode, to keep consistency with `show_incremental_sort_info`:

https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2890

Please review, thanks.

---

From: Julien Rouhaud <rjuju123@gmail.com>
Sent: Friday, March 25, 2022 17:04
To: Jian Guo <gjian@vmware.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>; Zhenghua Lyu <zlyu@vmware.com>
Subject: Re: Summary Sort workers Stats in EXPLAIN ANALYZE

 

⚠ External Email

Hi,

On Thu, Mar 24, 2022 at 07:50:11AM +0000, Jian Guo wrote:
> For a simple demo, with this explain statement：
>
> -- Test sort stats summary
> set force_parallel_mode=on;
> select explain_filter('explain (analyze, summary off, timing off, costs off, format json) select * from tenk1 order by unique1');
>
> Before this patch, we got plan like this:
>
>
>            "Node Type": "Sort",                +
>            "Parent Relationship": "Outer",     +
>            "Parallel Aware": false,            +
>            "Async Capable": false,             +
>            "Actual Rows": 10000,               +
>            "Actual Loops": 1,                  +
>            "Sort Key": ["unique1"],            +
>            "Workers": [                        +
>              {                                 +
>                "Worker Number": 0,             +
>                "Sort Method": "external merge",+
>                "Sort Space Used": 2496,        +
>                "Sort Space Type": "Disk"       +
>              }                                 +
>            ],                                  +

> After this patch, the effected plan is this:
>
>            "Node Type": "Sort",               +
>            "Parent Relationship": "Outer",    +
>            "Parallel Aware": false,           +
>            "Async Capable": false,            +
>            "Actual Rows": N,                  +
>            "Actual Loops": N,                 +
>            "Sort Key": ["unique1"],           +
>            "Workers planned": N,              +
>            "Sort Method": "external merge",   +
>            "Average Sort Space Used": N,      +
>            "Peak Sort Space Used": N,         +
>            "Sort Space Type": "Disk",         +

I think the idea is interesting, however there are a few problems in the patch.

First, I think that it should only be done in the VERBOSE OFF mode.  If you ask
for a VERBOSE output you don't need both the details and the summarized
version.

Other minor problems:

- why (only) emitting the number of workers planned and not the number of
  workers launched?
- the textual format is missing details about what the numbers are, which is
  particularly obvious since avgSpaceUsed and peakSpaceUsed don't have any unit
  or even space between them:

+                        "Sort Method: %s  %s: " INT64_FORMAT INT64_FORMAT "kB\n",
+                        sortMethod, spaceType, avgSpaceUsed, peakSpaceUsed);


________________________________

⚠ External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.