On 5 April 2016 at 13:09, David Rowley <david.row...@2ndquadrant.com> wrote: > On 5 April 2016 at 11:59, Robert Haas <robertmh...@gmail.com> wrote: >> One of my EDB colleagues, while in the process of refactoring some >> unrelated Advanced Server code, discovered that (1) there's no way to >> mark an aggregate as anything other than parallel-unsafe but (2) it >> doesn't matter because has_parallel_hazard ignores Aggrefs anyway. >> These mistakes cancel each other out (sorta) if all of your aggregates >> happen to be parallel-safe, but otherwise not so much. Barring >> objections, I intend to speedily apply the attached patch to fix this. > > Thanks for working on this. I should have noticed this myself... > > I had a quick look at this and I manged to make this happen; > > david=# create aggregate mysum(int) (sfunc=int4pl, combinefunc=int4pl, > stype=int, parallel); > server closed the connection unexpectedly > > I've attached a fix, which makes the code a bit more simple, and also > inline with the other code in DefineAggregate(). > > I think there was also a couple of missing syntax synopsis in the docs > too. I've added those.
Another thought; + else if (IsA(node, Aggref)) + { + Aggref *aggref = (Aggref *) node; + + if (parallel_too_dangerous(func_parallel(aggref->aggfnoid), context)) + return true; + } Does this need to check the parallel flags on the transfn or serialfn? these'll be executed on the worker process. Possibly we also need the combinefn/deserialfn/finalfn to be checked too as I see that we do generate_gather_paths() from set_append_rel_pathlist(). -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers