public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Richard Guo <[email protected]>
Cc: [email protected]
Subject: Re: A modest proposal: make parser/rewriter/planner inputs read-only
Date: Tue, 15 Apr 2025 13:31:44 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs4_9ozYZi4_s8DqVKDtOmYTmeRgQ0pZcZh1b4+3VxD8XXQ@mail.gmail.com>
References: <[email protected]>
	<CAMbWs4_9ozYZi4_s8DqVKDtOmYTmeRgQ0pZcZh1b4+3VxD8XXQ@mail.gmail.com>

Richard Guo <[email protected]> writes:
> On Sun, Apr 6, 2025 at 1:46 AM Tom Lane <[email protected]> wrote:
>> Years ago we fixed the executor to treat its input Plan trees as
>> read-only.  It seems like we really ought to do the same for these
>> upstream subsystems.  Surely, whatever benefit we get from changing
>> Node contents in-place is lost due to all these other hacks.

> While I'm in favor of this idea, it seems that it will require lots of
> code changes.  In particular, within the planner, the parsetree goes
> through considerable transformations during the preprocessing phase,
> such as subquery pull-up, constant folding, and so on.  Would this
> proposal mean we'd need to refactor all those places to treat the
> input parsetrees as read-only?  Just trying to understand the scope of
> what would be involved.

From a high-level point of view, I'd be satisfied to have this
property at the subsystem level: parse analysis can't modify the raw
parse tree output by the grammar, rewriter can't modify the analyzed
parse tree it's given, planner can't modify the parse tree it's given.
That would not preclude, say, portions of the planner doing
modify-in-place as long as it was certain that what they were
modifying was a planner-local copy.  (The proposed test infrastructure
would only be able to enforce things at the subsystem level anyway.)

As we get into the project we might decide that it'd be worth
enforcing similar rules on portions of those subsystems, eg successive
phases of the planner.  I don't have strong feelings about that at
present.  An example here is that it'd be nice to have some way of
verifying that a GEQO search step doesn't change any of the data
structures that are supposed to remain the same.

I do suspect that there are specific functions that will need to be
made strict about this, for example eval_const_expressions, but that
doesn't necessarily translate to a hard requirement across the board.

			regards, tom lane

view thread (2+ messages)

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: A modest proposal: make parser/rewriter/planner inputs read-only
  In-Reply-To: <[email protected]>

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

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