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

On 10 April 2014 22:52, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> 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?)
>

Ah, that's disappointing. If it can't be made to work for aggregates
with internal state types, then I think it loses most of it's value,
and I don't think it will be of much more use in the moving aggregate
case either.

> 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.
>

Agreed. Thanks for prototyping it though.

Regards,
Dean