On Wed, Dec 6, 2017 at 10:16 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Wed, Dec 6, 2017 at 1:14 AM, Robert Haas <robertmh...@gmail.com> wrote: >> On Tue, Dec 5, 2017 at 7:24 AM, Amit Kapila <amit.kapil...@gmail.com> 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 the above coding pattern create a problem, if ExecProcNode >>> throws an error and outer block catches it and continues execution >>> (consider the case of execution inside PL blocks)? >> >> I don't see what the problem is. The query that got aborted by the >> error wouldn't be sharing an EState with one that didn't. > > That's right. Ignore my comment, I got confused. Other than that, I > don't see any problem with the code as such apart from that it looks > slightly hacky. I think Thomas or someone needs to develop a patch on > the lines you have mentioned or what Thomas was trying to describe in > his email and see how it comes out.
Yeah, it is a bit hacky, but I can't see a another way to fix it without changing released APIs and it's only for one release and will certainly be unhackified in v11. For v11 I think we need to decide between: 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 nearby thread where someone had over a hundred Gather nodes and had to crank up max_connections to reserve enough DSM slots. Of course, optimising for that case doesn't make too much sense (I suspect multi-gather plans will become less likely with Parallel Append and Parallel Hash in the picture anyway), but it would reduce a bunch of duplicated work we do when it happens as well as scarce slot consumption. If we did that, then all of a sudden es_query_dsa would make sense again (ie it'd be whole-query scoped), and we could revert that hacky change. Or we could do both things anyway. -- Thomas Munro http://www.enterprisedb.com
