MIME-Version: 1.0
References: <2838538.1705692747@sss.pgh.pa.us>
 <CAJ7c6TMApZ62NOOk73_BmKkKtdhV+i=tyvXR52DAB-YVuFANDA@mail.gmail.com>
 <3791430.1705943243@sss.pgh.pa.us> <3831858.1705959108@sss.pgh.pa.us>
 <3511300.1708112330@sss.pgh.pa.us>
 <CAEZATCUQB3ybgBxEHXq28ihMS3+Bq-Gy8RraJpKGmVpjES6E3A@mail.gmail.com>
 <2506269.1710514705@sss.pgh.pa.us> <2982030.1710609907@sss.pgh.pa.us>
 <CAEZATCVa63moqwsi37a6LKj9mDsnbzEUw0KJYT8Qe2OZ-7V+Jg@mail.gmail.com>
 <3127158.1710685722@sss.pgh.pa.us> <3399753.1710704359@sss.pgh.pa.us>
In-Reply-To: <3399753.1710704359@sss.pgh.pa.us>
From: Dean Rasheed <dean.a.rasheed@gmail.com>
Date: Mon, 18 Mar 2024 14:29:21 +0000
Message-ID: 
 <CAEZATCULqMzbME2UEpybnBnp_U_6Yaa2Zm1BFuPFXKUq4-RXhw@mail.gmail.com>
Subject: Re: Improving EXPLAIN's display of SubPlan nodes
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Aleksander Alekseev <aleksander@timescale.com>,
 pgsql-hackers@lists.postgresql.org,
	Chantal Keller <chantal.keller@universite-paris-saclay.fr>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAEZATCULqMzbME2UEpybnBnp_U_6Yaa2Zm1BFuPFXKUq4-RXhw%40mail.gmail.com>
Precedence: bulk

On Sun, 17 Mar 2024 at 19:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Here's a cleaned-up version that seems to successfully resolve
> PARAM_EXEC references in all cases.  I haven't changed the
> "(plan_name).colN" notation, but that's an easy fix once we have
> consensus on the spelling.

I took a more detailed look at this and the code and doc changes all
look good to me.

There's a comment at the end of find_param_generator() that should
probably say "No generator found", rather than "No referent found".

The get_rule_expr() code could perhaps be simplified a bit, getting
rid of the show_subplan_name variable and moving the recursive calls
to get_rule_expr() to after the switch statement -- if testexpr is
non-NULL, print it, else print the subplan name probably works for all
subplan types.

The "colN" notation has grown on me, especially when you look at
examples like those in partition_prune.out with a mix of Param types.
Not using "$n" for 2 different purposes is good, and I much prefer
this to the original "FROM [HASHED] SubPlan N (returns ...)" notation.

> There are two other loose ends bothering me:
>
> 1.  I see that Gather nodes sometimes print things like
>
>    ->  Gather (actual rows=N loops=N)
>          Workers Planned: 2
>          Params Evaluated: $0, $1
>          Workers Launched: N
>
> I propose we nuke the "Params Evaluated" output altogether.

+1

> 2.  MULTIEXPR_SUBLINK subplans now result in EXPLAIN output like
>
>    ->  Result
>          Output: (SubPlan 1).col1, (SubPlan 1).col2, (SubPlan 1), i.tableoid, i.ctid
>
> The undecorated reference to (SubPlan 1) is fairly confusing, since
> it doesn't correspond to anything that will actually get output.
> I suggest that perhaps instead this should read
>
>          Output: (SubPlan 1).col1, (SubPlan 1).col2, IGNORE(SubPlan 1), i.tableoid, i.ctid
>
> or
>
>          Output: (SubPlan 1).col1, (SubPlan 1).col2, RESET(SubPlan 1), i.tableoid, i.ctid

I think "RESET()" or "RESCAN()" or something like that is better than
"INGORE()", because it indicates that it is actually doing something.
I don't really have a better idea. Perhaps not all uppercase though,
since that seems to go against the rest of the EXPLAIN output.

Regards,
Dean