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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Florian Pflug <fgp(at)phlo(dot)org>
Cc: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, 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-04-09 21:55:16
Message-ID: 27481.1397080516@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Florian Pflug <fgp(at)phlo(dot)org> writes:
> I was (and still am) not in favour of duplicating the whole quadruple of
> (state, initialvalue, transferfunction, finalfunction) because it seems
> excessive. In fact, I believed that doing this would probably be grounds for
> outright rejection of the patch, on the base of catalog bloat. And your
> initial response to this suggestion seemed to confirm this.

Well, I think it's much more likely that causing a performance penalty for
cases unrelated to window aggregates would lead to outright rejection :-(.
The majority of our users probably don't ever use window functions, but
for sure they've heard of SUM(). We can't penalize the non-window case.

Expanding pg_aggregate from 10 columns (as per patch) to 14 (as per this
suggestion) is a little annoying but it doesn't sound like a show stopper.
It seems reasonable to assume that the extra initval would be NULL in most
cases, so it's probably a net addition of 12 bytes per row.

> On Apr9, 2014, at 20:20 , Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> The patch has in fact already done that to a couple of basic aggregates like
>> sum(int4). Has anyone bothered to test what side-effects that has on
>> non-windowed aggregation performance?

> I'm pretty sure David Rowley did some benchmarking. The results should be
> in this thread somewhere I think, but they currently evade me... Maybe David
> can re-post, if he's following this...

I saw benchmarks addressing window aggregation, but none looking for
side-effects on plain aggregation.

> If we really go down that road (and I'm far from convinced), then maybe
> instead of having a bunch of additional fields, we could have separate
> entries in pg_aggregate for the two cases, with links between them.

That seems like a complete mess; in particular it would break the primary
key for pg_aggregate (aggfnoid), and probably break every existing query
that looks at pg_aggregate. Some extra fields would not break such
expectations (in fact we've added fields to pg_aggregate in the past).

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2014-04-09 22:03:28 bogus tsdict, tsparser, etc object identities
Previous Message Tom Lane 2014-04-09 21:40:34 Re: WIP patch (v2) for updatable security barrier views