pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: In ExecInitModifyTable, don't scribble on the source plan.
Date: Thu, 22 May 2025 18:28:58 +0000
Message-ID: <E1uIAf3-000I1r-2x@gemulon.postgresql.org> (raw)

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

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d376ab570ef95f2eae13a77cbd9ba21383f195f7

Modified Files
--------------
src/backend/commands/explain.c                |  4 +++
src/backend/executor/nodeModifyTable.c        | 10 +++---
src/backend/optimizer/plan/setrefs.c          |  7 ++--
src/test/regress/expected/partition_prune.out | 47 +++++++++++++++------------
src/test/regress/sql/partition_prune.sql      |  6 ++--
5 files changed, 41 insertions(+), 33 deletions(-)



Message-ID: <E1uIAf3-000I1r-2x@gemulon.postgresql.org>
Permalink:  ../E1uIAf3-000I1r-2x@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1uIAf3-000I1r-2x@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: In ExecInitModifyTable, don't scribble on the source plan.
  In-Reply-To: <E1uIAf3-000I1r-2x@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox