Re: Assert failure in parallel query due to SubPlan double visit for hash join

Hi Lukas,

> The assertion failure happens because ExecParallelInitializeDSM visits
> that Sort twice and tries to shm_toc_insert() the same plan_node_id
> key.

I confirmed this also on an assert build.

> The instrumentation issue happens because (1) for loops, we
> accumulate the worker instrumentation twice in
> ExecParallelReportInstrumentation and then again in
> ExecParallelRetrieveInstrumentation for the nodes underneath the
> shared SubPlan, (2) for "Sort Method" the leader and workers end up
> pointing at different (duplicate) SharedSortInfo slots and the leader
> looks at the empty one.

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.

Your patch looks fine to me. I just had a few small comments:

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.

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.

--
Sami Imseih
Amazon Web Services (AWS)