Hi Sami, On Wed, Oct 28, 2026 at 5:17 PM Sami Imseih <[email protected]> wrote: > In a non-assert build, this kind of case is definitely not easy to spot, > unless someone is really scrutinizing these parts of the plan.
Yeah, agreed - its also a somewhat unlikely plan to have, so I don't think this is seen much in practice, and if it is, people won't realize its wrong. > Your patch looks fine to me. I just had a few small comments: Thanks for reviewing! > 1/ Maybe rename planstate_tree_collect() to something like > planstate_tree_collect_distinct(). The important part here is that each > distinct PlanState appears in the returned list only once, so the name should > probably say that. Yeah, good point on naming - if we stay with this structure I'll adjust. > 2/ Some execParallel.c comments still read as if the functions recurse. For > example ExecParallelRetrieveInstrumentation(), > ExecParallelReportInstrumentation(), > and ExecParallelInitializeWorker() now only deal with a single > PlanState at a time. Good point - there are a few that clearly need updating (e.g. ExecParallelRetrieveInstrumentation jumps out), will revise. Thanks, Lukas -- Lukas Fittl
