Re: [PATCH] Negative Transition Aggregate Functions (WIP) - Mailing list pgsql-hackers

From Florian Pflug
Subject Re: [PATCH] Negative Transition Aggregate Functions (WIP)
Date
Msg-id 9CC92FE3-47A5-4D40-B112-4E91D15C786A@phlo.org
Whole thread Raw
In response to Re: [PATCH] Negative Transition Aggregate Functions (WIP)  (Florian Pflug <fgp@phlo.org>)
Responses Re: [PATCH] Negative Transition Aggregate Functions (WIP)  (Florian Pflug <fgp@phlo.org>)
Re: [PATCH] Negative Transition Aggregate Functions (WIP)  (Dean Rasheed <dean.a.rasheed@gmail.com>)
List pgsql-hackers
On Jan29, 2014, at 13:45 , Florian Pflug <fgp@phlo.org> wrote:
> In fact, I'm
> currently leaning towards just forbidding non-strict forward transition
> function with strict inverses, and adding non-NULL counters to the
> aggregates that then require them. It's really only the SUM() aggregates
> that are affected by this, I think.

I finally got around to doing that, and the results aren't too bad. The
attached patches required that the strictness settings of the forward and
reverse transition functions agree, and employ exactly the same NULL-skipping
logic we always had.

The only aggregates seriously affected by that change were SUM(int2) and
SUM(int4).

The SUM, AVG and STDDEV aggregates which use NumericAggState where
already mostly prepared for this - all they required were a few adjustments
to correctly handle the last non-NULL, non-NaN input being removed, and a few
additional PG_ARGISNULL calls for the inverse transition functions since they're
now non-strict. I've also modified them to unconditionally allocate the state
at the first call, instead upon seeing the first non-NULL input, but that isn't
strictly required. But without that, the state can have three classes of values -
SQL-NULL, NULL pointer and valid pointer, and that's just confusing...

SUM(int2) and SUM(int4) now simply use the same transition functions as
AVG(int2) and AVG(int4), which use an int8 array to track the sum of the inputs
and the number of inputs, plus a new final function int2int4_sum(). Previously,
they used a single int8 as their state type.

Since I was touching the code anyway, I removed some unnecessary inverse
transition functions - namely, int8_avg_accum_inv and numeric_avg_accum_inv. These
are completely identical to their non-avg cousins - the only difference between
the corresponding forward transition functions is whether they request computation
of sumX2 (i.e. the sum of squares of the inputs) or not.

I haven't yet updated the docs - it'll do that if and when there's consensus
about whether this is the way to go or not.

best regards,
Florian Pflug

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: narwhal and PGDLLIMPORT
Next
From: Alvaro Herrera
Date:
Subject: Re: narwhal and PGDLLIMPORT