Re: Using 128-bit integers for sum, avg and statistics aggregates

On Thu, Mar 12, 2015 at 6:23 PM, Andreas Karlsson <andreas@proxel.se> wrote:
> Fixed.

Did you intend to attach a patch here?

>> I think you should talk about the new thing first (just after the
>> extant, first sentence "Integer data types use Numeric..."). Refer to
>> where 128-bit integers are used and how, and only then the other stuff
>> (exceptions). After that, put the  PolyNumAggState struct definition
>> and basic functions. Then int2_accum() and so on.
>
>
> Good idea! Do you think the rewritten comment is clear enough, or do I need
> to go into more detail?
>
> /*
>  * 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.


-- 
Peter Geoghegan