On Thu, Jan 16, 2014 at 9:45 AM, Florian Pflug <fgp@phlo.org> wrote:
I'm currently thinking the best way forward is to get a basic patch
without any NUMERIC stuff committed, and to revisit this after that's done.
Attached is a version to that effect. The number of inverse transition functions is down to 8 from 36.
I managed to keep avg(interval) in there as it's all integer based division.
sum for all int types remain, plus sum for interval and cash. count(exp) and count(*) also naturally remain.
sum(bigint) became a bit weird as it uses numeric types internally, so I had to keep the do_numeric_discard() function to support it. Previously this function did NaNCount-- if it was removing a NaN numeric, but since I got rid of the NaNCounter as it was no longer required I just decided to throw an error if we encounter a numeric with NaN. I thought this was ok as the only place the function is being called the numeric value being passed is built from a bigint which obviously can't be NaN. It may be better to just get rid of do_numeric_discard() and stick thesub_var(&(state->sumX), newval, &(state->sumX)); into int8_avg_accum_inv(). I don't have a preference as to which as I think there's reasons for both ways. Perhaps even the name int8_avg_accum_inv() is misleading as we never use it for avg(bigint). Anyway, I'll leave it as is for now as I just can't decide which way is better.
Florian, I've not had a chance to look at your updated v2.2 patch yet sorry. I'll try and get some time tomorrow evening.
