On Wed, Dec 14, 2016 at 10:08 PM, Andres Freund <and...@anarazel.de> wrote: > On 2016-12-14 22:00:45 -0500, Robert Haas wrote: >> I took a look at Andres's patches today and saw that they can't really >> be applied as-is, because they "temporarily" change the master's >> ParallelWorkerNumber but have no provision for restoring it after an >> ERROR. > > Yea, that's not quite right. Although I'm not sure it really matters > that much, given that we're not continuing execution after an error. We > could just reset it at appropriate points - but that seems ugly.
Yes. >> I think that approach isn't right anyway; it seems to me that >> what we want to do is set hash_iv based on we're in a Partial HashAgg, >> not whether we're in a parallel worker. For instance, if we're >> somehow in a nodeSubplan.c there's no need for this just because we >> happen to be in a worker -- I think. That led me to develop the >> attached patch. > > Hm, I'd rather have this be a more general solution - it doesn't seem > unlikely that other parts of the system want to know whether they're > currently executing a parallel portion of the plan. E.g. it really > should be forbidden to do modifications even in the parallel portion of > the plan the master executes. Right now that'd escape notice, which I > don't think is good. Actually, that wouldn't escape notice. You can test with IsInParallelMode(). That's entered before beginning execution of the plan at the outermost level - see ExecutePlan(). >> This may not be perfect, but it causes TPC-H Q18 to complete instead >> of hanging forever, so I'm going to commit it RSN unless there are >> loud objections combined with promising steps toward some alternative >> fix. It's been over a month since these problems were reported, and >> it is not good that the tree has been in a broken state for that >> entire time. > > Yea, sorry for that :(. Unfortunately the JIT stuff currently occupies a > large portion of my brain. > > I really hope to come up with a new version of the patch that does the > "smarter" expansion soon. I've got no problem with that at all, but I want to unbreak things more or less immediately and then you/we can further improve it later. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
