Horton

Re: Violation of principle that plan trees are read-only

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-05-20T20:40:34Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

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

  2. Rely on executor utils to build targetlist for DML RETURNING.

Attachments

Hi,

On 2025-05-20 16:18:57 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> >> I'm tempted to back-patch this: the plan tree damage seems harmless at
> >> present, but maybe it'd become less harmless with future fixes.
> 
> > There are *some* cases where this changes the explain output, but but the new
> > output is more correct, I think:
> > ...
> > I suspect this is an argument for backpatching, not against - seems that
> > deparsing could end up creating bogus output in cases where it could matter?
> > Not sure if such cases are reachable via views (and thus pg_dump) or
> > postgres_fdw, but it seems possible.
> 
> I don't believe that we guarantee EXPLAIN output to be 100% valid SQL,
> so I doubt there's a correctness argument here; certainly it'd not
> affect pg_dump.

I wasn't thinking of EXPLAIN itself, but was wondering whether it's possible
to create a view, rule or such that is affected by the output change. Would be
a weird case, if it existed.


> I'm curious though: what was the test case you were looking at?

It's a modified query from our regression tests, I had added some debugging to
find cases where the targetlists differed.  I attached the extracted, somewhat
modified, sql script.

Greetings,

Andres Freund