Re: [PATCH] Negative Transition Aggregate Functions (WIP)

On Thu, Jan 16, 2014 at 9:45 AM, Florian Pflug <fgp(at)phlo(dot)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 the sub_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.

Regards

David Rowley

> best regards,
> Florian Pflug
>
>