On 14/03/15 04:17, Andreas Karlsson wrote:
> On 03/13/2015 10:22 PM, Peter Geoghegan wrote:
>> On Thu, Mar 12, 2015 at 6:23 PM, Andreas Karlsson <andreas@proxel.se>
>> wrote:
>>> /*
>>> * Integer data types use Numeric accumulators to share code and
>>> avoid risk
>>> * of overflow. To speed up aggregation 128-bit integer
>>> accumulators are
>>> * used instead where sum(X) or sum(X*X) fit into 128-bits, and
>>> there is
>>> * platform support.
>>> *
>>> * For int2 and int4 inputs sum(X) will fit into a 64-bit
>>> accumulator, hence
>>> * we use faster special-purpose accumulator routines for SUM and
>>> AVG of
>>> * these datatypes.
>>> */
>>>
>>> #ifdef HAVE_INT128
>>> typedef struct Int128AggState
>>
>> Not quite. Refer to the 128-bit integer accumulators as
>> "special-purpose accumulator routines" instead. Then, in the case of
>> the extant 64-bit accumulators, refer to them by the shorthand
>> "integer accumulators". Otherwise it's the wrong way around.
>
> I disagree. The term "integer accumulators" is confusing. Right now I do
> not have any better ideas for how to write that comment, so I submit the
> next version of the patch with the comment as I wrote it above. Feel
> free to come up with a better wording, my English is not always up to
> par when writing technical texts.
>
I don't like the term "integer accumulators" either as "integer" is
platform specific. I would phase it like this:
/* * Integer data types in general use Numeric accumulators to share code * and avoid risk of overflow. * * However for
performancereasons some of the optimized special-purpose * accumulator routines are used when possible. * * On
platformswith 128-bit integer support, the 128-bit routines will be * used when sum(X) or sum(X*X) fit into 128-bit. *
*For int2 and int4 inputs, the N and sum(X) fit into 64-bit so the 64-bit * accumulators will be used for SUM and AVG
ofthese data types. */
It's almost the same thing as you wrote but the 128 bit and 64 bit
optimizations are put on the same "level" of optimized routines.
But this is nitpicking at this point, I am happy with the patch as it
stands right now.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services