On Mon, Mar 9, 2015 at 5:37 PM, Andreas Karlsson <andr...@proxel.se> wrote: > int128-agg-v7.patch
I see a spelling error: "+ * On platforms which support 128-bit integers some aggergates instead use a" Other than that, the patch looks pretty good to me. You're generalizing from the example of existing routines for int128_to_numericvar(), and other such code in a fairly mechanical way. The performance improvements are pretty real and tangible. You say: /* * Integer data types use Numeric accumulators to share code and avoid * risk of overflow. For int2 and int4 inputs, Numeric accumulation * is overkill for the N and sum(X) values, but definitely not overkill * for the sum(X*X) value. Hence, we use int2_accum and int4_accum only * for stddev/variance --- there are faster special-purpose accumulator * routines for SUM and AVG of these datatypes. * * Similarily we can, where available, use 128-bit integer accumulators * for sum(X) for int8 and sum(X*X) for int2 and int4, but not sum(X*X) * for int8. */ 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. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
