In ExecInitModifyTable, don't scribble on the source plan.

The code carelessly modified mtstate->ps.plan->targetlist,
which it's not supposed to do.  Fortunately, there's not
really any need to do that because the planner already
set up a perfectly acceptable targetlist for the plan node.
We just need to remove the erroneous assignments and update some
relevant comments.

As it happens, the erroneous assignments caused the targetlist to
point to a different part of the source plan tree, so that there
isn't really a risk of the pointer becoming dangling after executor
termination.  The only visible effect of this change we can find is
that EXPLAIN will show upper references to the ModifyTable's output
expressions using different variables.  Formerly it showed Vars from
the first target relation that survived executor-startup pruning.
Now it always shows such references using the first relation appearing
in the planner output, independently of what happens during executor
pruning.  On the whole that seems like a good thing.

Also make a small tweak in ExplainPreScanNode to ensure that the first
relation will receive a refname assignment in set_rtable_names, even
if it got pruned at startup.  Previously the Vars might be shown
without any table qualification, which is confusing in a multi-table
query.

I considered back-patching this, but since the bug doesn't seem to
have any really terrible consequences in existing branches, it
seems better to not change their EXPLAIN output.  It's not too late
for v18 though, especially since v18 already made other changes in
the EXPLAIN output for these cases.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/213261.1747611093@sss.pgh.pa.us

src/backend/commands/explain.c

@@ -1220,6 +1220,10 @@ ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used)
 			if (((ModifyTable *) plan)->exclRelRTI)
 				*rels_used = bms_add_member(*rels_used,
 											((ModifyTable *) plan)->exclRelRTI);
+			/* Ensure Vars used in RETURNING will have refnames */
+			if (plan->targetlist)
+				*rels_used = bms_add_member(*rels_used,
+											linitial_int(((ModifyTable *) plan)->resultRelations));
 			break;
 		case T_Append:
 			*rels_used = bms_add_members(*rels_used,

src/backend/executor/nodeModifyTable.c

@@ -4830,12 +4830,11 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 		ExprContext *econtext;
 
 		/*
-		 * Initialize result tuple slot and assign its rowtype using the first
+		 * Initialize result tuple slot and assign its rowtype using the plan
-		 * RETURNING list.  We assume the rest will look the same.
+		 * node's declared targetlist, which the planner set up to be the same
+		 * as the first (before runtime pruning) RETURNING list.  We assume
+		 * all the result rels will produce compatible output.
 		 */
-		mtstate->ps.plan->targetlist = (List *) linitial(returningLists);
-
-		/* Set up a slot for the output of the RETURNING projection(s) */
 		ExecInitResultTupleSlotTL(&mtstate->ps, &TTSOpsVirtual);
 		slot = mtstate->ps.ps_ResultTupleSlot;
 
@@ -4865,7 +4864,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 		 * We still must construct a dummy result tuple type, because InitPlan
 		 * expects one (maybe should change that?).
 		 */
-		mtstate->ps.plan->targetlist = NIL;
 		ExecInitResultTypeTL(&mtstate->ps);
 
 		mtstate->ps.ps_ExprContext = NULL;

src/backend/optimizer/plan/setrefs.c

@@ -1097,9 +1097,10 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 
 					/*
 					 * Set up the visible plan targetlist as being the same as
-					 * the first RETURNING list. This is for the use of
+					 * the first RETURNING list.  This is mostly for the use
-					 * EXPLAIN; the executor won't pay any attention to the
+					 * of EXPLAIN; the executor won't execute that targetlist,
-					 * targetlist.  We postpone this step until here so that
+					 * although it does use it to prepare the node's result
+					 * tuple slot.  We postpone this step until here so that
 					 * we don't have to do set_returning_clause_references()
 					 * twice on identical targetlists.
 					 */

src/test/regress/expected/partition_prune.out

@@ -4553,16 +4553,18 @@ create view part_abc_view as select * from part_abc where b <> 'a' with check op
 prepare update_part_abc_view as update part_abc_view set b = $2 where a = $1 returning *;
 -- Only the unpruned partition should be shown in the list of relations to be
 -- updated
-explain (costs off) execute update_part_abc_view (1, 'd');
+explain (verbose, costs off) execute update_part_abc_view (1, 'd');
                                  QUERY PLAN                                  
--------------------------------------------------------
+-----------------------------------------------------------------------------
- Update on part_abc
+ Update on public.part_abc
-   Update on part_abc_1
+   Output: part_abc_1.a, part_abc_1.b, part_abc_1.c
+   Update on public.part_abc_1
    ->  Append
          Subplans Removed: 1
-         ->  Seq Scan on part_abc_1
+         ->  Seq Scan on public.part_abc_1
-               Filter: ((b <> 'a'::text) AND (a = $1))
+               Output: $2, part_abc_1.tableoid, part_abc_1.ctid
-(6 rows)
+               Filter: ((part_abc_1.b <> 'a'::text) AND (part_abc_1.a = $1))
+(8 rows)
 
 execute update_part_abc_view (1, 'd');
  a | b | c 
@@ -4570,28 +4572,31 @@ execute update_part_abc_view (1, 'd');
  1 | d | t
 (1 row)
 
-explain (costs off) execute update_part_abc_view (2, 'a');
+explain (verbose, costs off) execute update_part_abc_view (2, 'a');
                                  QUERY PLAN                                  
--------------------------------------------------------
+-----------------------------------------------------------------------------
- Update on part_abc
+ Update on public.part_abc
-   Update on part_abc_2 part_abc_1
+   Output: part_abc_1.a, part_abc_1.b, part_abc_1.c
+   Update on public.part_abc_2
    ->  Append
          Subplans Removed: 1
-         ->  Seq Scan on part_abc_2 part_abc_1
+         ->  Seq Scan on public.part_abc_2
-               Filter: ((b <> 'a'::text) AND (a = $1))
+               Output: $2, part_abc_2.tableoid, part_abc_2.ctid
-(6 rows)
+               Filter: ((part_abc_2.b <> 'a'::text) AND (part_abc_2.a = $1))
+(8 rows)
 
 execute update_part_abc_view (2, 'a');
 ERROR:  new row violates check option for view "part_abc_view"
 DETAIL:  Failing row contains (2, a, t).
 -- All pruned.
-explain (costs off) execute update_part_abc_view (3, 'a');
+explain (verbose, costs off) execute update_part_abc_view (3, 'a');
                      QUERY PLAN                     
------------------------------
+----------------------------------------------------
- Update on part_abc
+ Update on public.part_abc
+   Output: part_abc_1.a, part_abc_1.b, part_abc_1.c
    ->  Append
          Subplans Removed: 2
-(3 rows)
+(4 rows)
 
 execute update_part_abc_view (3, 'a');
  a | b | c 

src/test/regress/sql/partition_prune.sql

@@ -1371,12 +1371,12 @@ create view part_abc_view as select * from part_abc where b <> 'a' with check op
 prepare update_part_abc_view as update part_abc_view set b = $2 where a = $1 returning *;
 -- Only the unpruned partition should be shown in the list of relations to be
 -- updated
-explain (costs off) execute update_part_abc_view (1, 'd');
+explain (verbose, costs off) execute update_part_abc_view (1, 'd');
 execute update_part_abc_view (1, 'd');
-explain (costs off) execute update_part_abc_view (2, 'a');
+explain (verbose, costs off) execute update_part_abc_view (2, 'a');
 execute update_part_abc_view (2, 'a');
 -- All pruned.
-explain (costs off) execute update_part_abc_view (3, 'a');
+explain (verbose, costs off) execute update_part_abc_view (3, 'a');
 execute update_part_abc_view (3, 'a');
 deallocate update_part_abc_view;