Re: es_query_dsa is broken

On Thu, Nov 30, 2017 at 11:19 PM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> On Thu, Nov 30, 2017 at 4:01 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> Better ideas?
>>
>> How about this:
>>
>> 1. Remove es_query_dsa altogether.
>> 2. Add a dsa_area * to ExecParallelInitializeDSMContext.
>> 3. In ExecParallelInitializeDSM, pass the dsa_area * as a separate to
>> the per-node-type function.
>> 4. If the per-node-type function cares about the dsa_area *, it is
>> responsible for saving a pointer to it in the PlanState node.
>> 5. Also pass it down via the ParallelWorkerContext.
>>
>> In v10 we might need to go with a solution like what you've sketched
>> here, because Tom will complain about breaking binary compatibility
>> with EState (and maybe other PlanState nodes) in a released branch.

Here's a pair of versions of that patch tidied up a bit, for
REL_10_STABLE and master. Unfortunately I've been unable to come up
with a reproducer that shows misbehaviour (something like what was
reported on -bugs[1] which appears to be a case of calling
dsa_get_address(wrong_area, some_dsa_pointer)).

I don't yet have a patch to remove es_query_dsa. Instead of adding a
dsa_area * parameter to a ton of per-node functions as you suggested,
I'm wondering if we shouldn't just pass the whole ParallelExecutorInfo
object into all ExecXXXInitializeDSM() and ExecXXXInitializeWorker()
functions so they can hold onto it if they want to. In the worker
case it'd be only partially filled out: just area and pcxt, and pcxt
would also be only partially filled out. Alternatively I could create
a new struct ParallelExecutorWorkerInfo which has just the bit needed
for workers (that'd replace ParallelWorkerContext, which I now realise
was put in the wrong place -- it doesn't belong in access/parallel.h,
it belongs in an executor header with an executor-sounding name). I'd
like to get a couple of other things done before I come back to this.

[1] https://www.postgresql.org/message-id/CAEepm=1V-ZjAAyG-xj6s7ESXFhzLsRBpyX=BLz-w2DS=ObNu4A@mail.gmail.com

--
Thomas Munro
http://www.enterprisedb.com