Re: es_query_dsa is broken

2018-04-11 Thread Thomas Munro
On Thu, Apr 12, 2018 at 4:04 AM, Andres Freund wrote: > This is an open item for v11: > > Tidy up es_query_dsa and possibly ParallelWorkerContext? > Original commit: e13029a5ce353574516c64fd1ec9c50201e705fd (principal > author: Thomas Munro; owner: Robert Haas) > Bug fix: fd7

Re: es_query_dsa is broken

2018-04-11 Thread Andres Freund
Hi, On 2017-12-07 12:51:56 +1300, Thomas Munro wrote: > 1. Removing es_query_dsa and injecting the right context into the > executor tree as discussed. > > 2. Another idea mentioned by Robert in an off-list chat: We could > consolidate all DSM segments in a multi-gather plan into one. See the

Re: es_query_dsa is broken

2017-12-18 Thread Robert Haas
On Sun, Dec 17, 2017 at 7:35 PM, Thomas Munro wrote: > Andreas Seltenreich found a query[1] that suffers from this bug > (thanks!), and Amit confirmed that this patch fixes it, but further > sqlsmith fuzz testing with the patch applied also revealed that it > failed to handle the case where pei is

Re: es_query_dsa is broken

2017-12-17 Thread Thomas Munro
On Thu, Dec 7, 2017 at 12:51 PM, Thomas Munro wrote: > On Wed, Dec 6, 2017 at 10:16 PM, Amit Kapila wrote: >> On Wed, Dec 6, 2017 at 1:14 AM, Robert Haas wrote: >>> On Tue, Dec 5, 2017 at 7:24 AM, Amit Kapila wrote: + EState *estate = gatherstate->ps.state; + + /* Install our DSA

Re: es_query_dsa is broken

2017-12-06 Thread Thomas Munro
On Wed, Dec 6, 2017 at 10:16 PM, Amit Kapila wrote: > On Wed, Dec 6, 2017 at 1:14 AM, Robert Haas wrote: >> On Tue, Dec 5, 2017 at 7:24 AM, Amit Kapila wrote: >>> + EState *estate = gatherstate->ps.state; >>> + >>> + /* Install our DSA area while executing the plan. */ >>> + estate->es_query_dsa

Re: es_query_dsa is broken

2017-12-06 Thread Amit Kapila
On Wed, Dec 6, 2017 at 1:14 AM, Robert Haas wrote: > On Tue, Dec 5, 2017 at 7:24 AM, Amit Kapila wrote: >> + EState *estate = gatherstate->ps.state; >> + >> + /* Install our DSA area while executing the plan. */ >> + estate->es_query_dsa = gatherstate->pei->area; >> outerTupleSlot = ExecProcNod

Re: es_query_dsa is broken

2017-12-05 Thread Robert Haas
On Tue, Dec 5, 2017 at 7:24 AM, Amit Kapila wrote: > + EState *estate = gatherstate->ps.state; > + > + /* Install our DSA area while executing the plan. */ > + estate->es_query_dsa = gatherstate->pei->area; > outerTupleSlot = ExecProcNode(outerPlan); > + estate->es_query_dsa = NULL; > > Won't th

Re: es_query_dsa is broken

2017-12-05 Thread Amit Kapila
On Tue, Dec 5, 2017 at 6:45 AM, Thomas Munro wrote: > On Thu, Nov 30, 2017 at 11:19 PM, Thomas Munro > wrote: >> On Thu, Nov 30, 2017 at 4:01 AM, Robert Haas wrote: >>> >>> In v10 we might need to go with a solution like what you've sketched >>> here, because Tom will complain about breaking bin

Re: es_query_dsa is broken

2017-12-04 Thread Thomas Munro
On Thu, Nov 30, 2017 at 11:19 PM, Thomas Munro wrote: > On Thu, Nov 30, 2017 at 4:01 AM, Robert Haas 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

Re: es_query_dsa is broken

2017-11-30 Thread Thomas Munro
On Thu, Nov 30, 2017 at 4:01 AM, Robert Haas 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 t

Re: es_query_dsa is broken

2017-11-29 Thread Robert Haas
On Wed, Nov 29, 2017 at 7:30 AM, Thomas Munro wrote: > While reviewing commit c6755e23 I realised that es_query_dsa is > broken. It might have made some kind of sense as a name and a concept > in an earlier version of the proposal to add a DSA area for parallel > query's use, when the DSA area wa