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

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

Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> writes:
> I was imagining that firsttrans would only be passed the first value
> to be aggregated, not any previous state, and that it would be illegal
> to specify both an initcond and a firsttrans function.

> The forward transition function would only be called for values after
> the first, by which point the state would be non-null, and so it could
> be made strict in most cases. The same would apply to the invertible
> transition functions, so they wouldn't have to do null counting, which
> in turn would make their state types simpler.

I put together a very fast proof-of-concept patch for this (attached).
It has a valid execution path for an aggregate with initfunc, but I didn't
bother writing the CREATE AGGREGATE support yet. I made sum(int4) work
as you suggest, marking the transfn strict and ripping out int4_sum's
internal support for null inputs. The result seems to be about a 4% or so
improvement in the overall aggregation speed, for a simple "SELECT
sum(int4col) FROM table" query. So from a performance standpoint this
seems only marginally worth doing. The real problem is not that 4% isn't
worth the trouble, it's that AFAICS the only built-in aggregates that
can benefit are sum(int2) and sum(int4). So that looks like a rather
narrow use-case.

You had suggested upthread that we could use this idea to make the
transition functions strict for aggregates using "internal" transition
datatypes, but that does not work because the initfunc would violate
the safety rule that a function returning internal must take at least
one internal-type argument. That puts a pretty strong damper on the
usefulness of the approach, given how many internal-transtype aggregates
we have (and the moving-aggregate case is not going to be better is it?)

So at this point I'm feeling unexcited about the initfunc idea.
Unless it does something really good for the moving-aggregate case,
I think we should drop it.

regards, tom lane

Attachment Content-Type Size
aggregate-initfunc-prototype.patch text/x-diff 34.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Florian Pflug 2014-04-10 21:54:33 Re: [PATCH] Negative Transition Aggregate Functions (WIP)
Previous Message Bruce Momjian 2014-04-10 21:44:26 Re: PostgreSQL in Windows console and Ctrl-C