On 17 March 2016 at 16:30, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> On Wed, Mar 16, 2016 at 10:08 PM, David Rowley
> <david.rowley@2ndquadrant.com> wrote:
>> On 16 March 2016 at 23:54, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
>>> On Wed, Mar 16, 2016 at 8:34 AM, David Rowley
>>> <david.rowley@2ndquadrant.com> wrote:
>>>> 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.
>>>
>>> The additional combine function patch that I posted handles all float4 and
>>> float8 aggregates. There is an OID conflict with the latest source code,
>>> I will update the patch and post it in that thread.
>>
>> Thanks! I just send a series of patches which add a whole host of
>> serial/deserial functions, and a patch which adds some documentation.
>> Maybe you could base your patch on the 0005 patch, and update the
>> documents to remove the "All types apart from floating-point types"
>> text and replace that with "Yes".
>
> Here I attached updated float aggregates patch based on 0005 patch.
Great! Thanks for sending that.
I just had a quick skim over the patch and noticed the naming
convention you're using for the combine function is *_pl, and you have
float8_pl. There's already a function named float8pl() which is quite
close to what you have. I've been sticking to *_combine() for these,
so maybe float8_combine() and float8_regr_combine() are better names.
-- David Rowley http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services