On Fri, Jan 20, 2023 at 12:58 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Amit Langote <amitlangot...@gmail.com> writes: > > On Fri, Jan 20, 2023 at 12:31 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > >> It might be possible to incorporate this pointer into PlannedStmt > >> instead of passing it separately. > > > Yeah, that would be less churn. Though, I wonder if you still hold > > that PlannedStmt should not be scribbled upon outside the planner as > > you said upthread [1]? > > Well, the whole point of that rule is that the executor can't modify > a plancache entry. If the plancache itself sets a field in such an > entry, that doesn't seem problematic from here. > > But there's other possibilities if that bothers you; QueryDesc > could hold the field, for example. Also, I bet we'd want to copy > it into EState for the main initialization recursion.
QueryDesc sounds good to me, and yes, also a copy in EState in any case. So I started looking at the call sites of CreateQueryDesc() and stopped to look at ExecParallelGetQueryDesc(). AFAICS, we wouldn't need to pass the CachedPlan to a parallel worker's rerun of InitPlan(), because 1) it doesn't make sense to call the plancache in a parallel worker, 2) the leader should already have taken all the locks in necessary for executing a given plan subnode that it intends to pass to a worker in ExecInitGather(). Does that make sense? -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
