From: Tom Lane <tgl@sss.pgh.pa.us>
To: Aleksander Alekseev <aleksander@timescale.com>
cc: pgsql-hackers@lists.postgresql.org,
        Chantal Keller <chantal.keller@universite-paris-saclay.fr>
Subject: Re: Improving EXPLAIN's display of SubPlan nodes
In-reply-to: <3791430.1705943243@sss.pgh.pa.us>
References: <2838538.1705692747@sss.pgh.pa.us>
 <CAJ7c6TMApZ62NOOk73_BmKkKtdhV+i=tyvXR52DAB-YVuFANDA@mail.gmail.com>
 <3791430.1705943243@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 22 Jan 2024 12:07:23 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <3831842.1705959098.0@sss.pgh.pa.us>
Date: Mon, 22 Jan 2024 16:31:48 -0500
Message-ID: <3831858.1705959108@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3831858.1705959108%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3831842.1705959098.1@sss.pgh.pa.us>

I wrote:
> The main thing that's still missing compared to what is in the plan
> data structure is information about which Param is which.  I think
> we have the subplan output Params relatively well covered through
> the expedient of listing them in the generated plan_name, but it's
> still not apparent to me how we could shoehorn subplan input
> Params into this (or whether it's worth doing).

Actually ... it looks like it probably isn't worth doing, because
it's already the case that we don't expose input Params as such.
EXPLAIN searches for the referent of an input Param and displays
that (cf find_param_referent()).  Just for experimental purposes,
I wrote a follow-on patch to add printout of the parParam and args
list (attached, as .txt so the cfbot doesn't think it's a patch).
This produces results like

explain (verbose, costs off)
select array(select sum(x+y) s
            from generate_series(1,3) y group by y order by s)
  from generate_series(1,3) x;
                            QUERY PLAN                             
-------------------------------------------------------------------
 Function Scan on pg_catalog.generate_series x
   Output: ARRAY(SubPlan 1 PASSING $0 := x.x)
                           ^^^^^^^^^^^^^^^^^ added by delta patch
   Function Call: generate_series(1, 3)
   SubPlan 1
     ->  Sort
           Output: (sum((x.x + y.y))), y.y
           Sort Key: (sum((x.x + y.y)))
           ->  HashAggregate
                 Output: sum((x.x + y.y)), y.y
                              ^^^ actual reference to $0
                 Group Key: y.y
                 ->  Function Scan on pg_catalog.generate_series y
                       Output: y.y
                       Function Call: generate_series(1, 3)
(13 rows)

As you can see, it's not necessary to explain what $0 is because
that name isn't shown anywhere else --- the references to "x.x" in
the subplan are actually uses of $0.

So now I'm thinking that we do have enough detail in the present
proposal, and we just need to think about whether there's some
nicer way to present it than the particular spelling I used here.

One idea I considered briefly is to pull the same trick with
regards to output parameters --- that is, instead of adding all
the "returns $n" annotations to subplans, drill down and print
the subplan's relevant targetlist expression instead of "$n".
On balance I think that might be more confusing not less so,
though.  SQL users are used to the idea that a sub-select can
"see" variables from the outer query, but not at all vice-versa.
I think it probably wouldn't be formally ambiguous, because
ruleutils already de-duplicates table aliases across the whole
tree, but it still seems likely to be confusing.  Also, people
are already pretty used to seeing $n to represent the outputs
of InitPlans, and I've not heard many complaints suggesting
that we should change that.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/plain; name="report-subplan-input-params-delta.txt";
	charset="us-ascii"
Content-ID: <3831842.1705959098.2@sss.pgh.pa.us>
Content-Description: report-subplan-input-params-delta.txt
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/rul=
eutils.c
index 9ee35640ba..b3da98a2f7 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -8911,9 +8911,30 @@ get_rule_expr(Node *node, deparse_context *context,
 						break;
 				}
 				if (subplan->useHashTable)
-					appendStringInfo(buf, "HASHED %s)", subplan->plan_name);
+					appendStringInfo(buf, "HASHED %s", subplan->plan_name);
 				else
-					appendStringInfo(buf, "%s)", subplan->plan_name);
+					appendStringInfo(buf, "%s", subplan->plan_name);
+				if (subplan->parParam)
+				{
+					ListCell   *lc3;
+					ListCell   *lc4;
+					bool first =3D true;
+
+					appendStringInfoString(buf, " PASSING ");
+					forboth(lc3, subplan->parParam, lc4, subplan->args)
+					{
+						int			paramid =3D lfirst_int(lc3);
+						Node	   *arg =3D (Node *) lfirst(lc4);
+
+						if (first)
+							first =3D false;
+						else
+							appendStringInfoString(buf, ", ");
+						appendStringInfo(buf, "$%d :=3D ", paramid);
+						get_rule_expr(arg, context, showimplicit);
+					}
+				}
+				appendStringInfoChar(buf, ')');
 			}
 			break;
 =


------- =_aaaaaaaaaa0--