Neil Conway <neilc@samurai.com> writes:
> I just noticed that there is a `PlanState' node in the executor, of all
> places. I'm thinking of using `QueryState' instead -- this parallels the
> usage of PlanState in the executor, to some degree (PlanState holds some
> of the state of the executor as it examines a Plan; QueryState holds
> some of the state of the planner as it examines a Query). I also
> considered `PlannerState', but that doesn't seem best, as there can be
> multiple instances of this struct in existence for a single invocation
> of planner() (due to subqueries).
I'd go with PlannerState. QueryState for some reason sounds more like
execution-time state. I don't buy the notion that PlannerState sounds
like something there should be only one of; or at least, by the time
you've found out that the planner is potentially re-entrant, that should
not bother you ...
I'm not really sure what to do about the problem ofplan->foo = process(plan->foo)
... it's just *such* a convenient notation. Maybe it would be
sufficient to legislate that you can only do that when you know that you
are working with a copied "plan" object. The various recursive
"process" routines can be designed to return fresh structure whenever
they change anything --- for the most part we do that already, I think.
So this might just boil down to having to make a "flat" copy of the
top-level Query node before we start whacking at it.
Pulling the "planner internal" stuff out of the Query node does seem
like a good idea, even so.
regards, tom lane
