Richard Guo <guofengli...@gmail.com> writes:
> On Sun, Apr 6, 2025 at 1:46 AM Tom Lane <t...@sss.pgh.pa.us> 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
