On 16 March 2016 at 06:39, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > After looking at the parallel aggregate patch, I also looked at this one, as > it's naturally related. Sadly I haven't found any issue that I could nag > about ;-) The patch seems well baked, as it was in the oven for quite a long > time already.
Thanks for looking at this. > The one concern I do have is that it only adds (de)serialize functions for > SUM(numeric) and AVG(numeric). I think it's reasonable not to include that > into the patch, but it will look a bit silly if that's all that gets into > 9.6. Yes me too, so I spent several hours yesterday writing all of the combine functions and serialisation/deserialisation that are required for all of SUM(), AVG() STDDEV*(). I also noticed that I had missed using some existing functions for bool_and() and bool_or() so I added those to pg_aggregate.h. I'm just chasing down a crash bug on HAVE_INT128 enabled builds, so should be posting a patch quite soon. I didn't touch the FLOAT4 and FLOAT8 aggregates as I believe Haribabu has a patch for that over on the parallel aggregate thread. I've not looked at it in detail yet. If Haribabu's patch does all that's required for the numerical aggregates for floating point types then the status of covered aggregates is (in order of pg_aggregate.h): * AVG() complete coverage * SUM() complete coverage * MAX() complete coverage * MIN() complete coverage * COUNT() complete coverage * STDDEV + friends complete coverage * regr_*,covar_pop,covar_samp,corr not touched these. * bool*() complete coverage * bitwise aggs. complete coverage * Remaining are not touched. I see diminishing returns with making these parallel for now. I think I might not be worth pushing myself any harder to make these ones work. Does what I have done + floating point aggs from Haribabu seem reasonable for 9.6? > I think it would be good to actually document which aggregates support > parallel execution, and which don't. Currently the docs don't mention this > at all, and it's tricky for regular users to deduce that as it's related to > the type of the internal state and not to the input types. An example of > that is the SUM(bigint) example mentioned above. I agree. I will look for a suitable place. -- 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
