On Sun, 17 Mar 2024 at 19:39, Tom Lane <t...@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