From: | Florian Pflug <fgp(at)phlo(dot)org> |
---|---|
To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Cc: | David Rowley <dgrowleyml(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kevin Grittner <kgrittn(at)ymail(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Greg Stark <stark(at)mit(dot)edu>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Negative Transition Aggregate Functions (WIP) |
Date: | 2014-02-21 11:54:19 |
Message-ID: | 169C29DD-DD32-4D49-97FD-ACA18C6B3612@phlo.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Feb20, 2014, at 02:48 , Florian Pflug <fgp(at)phlo(dot)org> wrote:
> On Jan29, 2014, at 13:45 , Florian Pflug <fgp(at)phlo(dot)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.
I realized only after posting this that the patches no longer apply to current HEAD.
Attached are rebased patches.
best regards,
Florian Pflug
Attachment | Content-Type | Size |
---|---|---|
invtrans_strictstrict_arith_3dd64e.patch | application/octet-stream | 66.9 KB |
invtrans_strictstrict_base_70f89c6.patch | application/octet-stream | 101.5 KB |
invtrans_strictstrict_collecting_be6a58.patch | application/octet-stream | 32.8 KB |
invtrans_strictstrict_docs_0cb944.patch | application/octet-stream | 22.1 KB |
invtrans_strictstrict_minmax_2a0dc2.patch | application/octet-stream | 70.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Florian Pflug | 2014-02-21 12:15:01 | Re: Proposal: IMPORT FOREIGN SCHEMA statement. |
Previous Message | Andres Freund | 2014-02-21 11:40:51 | Re: Changeset Extraction v7.6.1 |