Re: Window-functions patch handling of aggregates

From: Greg Stark <greg(dot)stark(at)enterprisedb(dot)com>
To: Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Gregory Stark <stark(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Window-functions patch handling of aggregates
Date: 2008-12-25 09:08:29
Message-ID: 64E68308-4279-4F2A-B075-A41480C3E327@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Yeah, it seems like adding a flag like iswindowable to aggregate
functions is the safest option.

It would be nice if it represented an abstract property of the state
function or final function rather than just "works with the
implementation of window functions". I'm not sure what that property
is though - isidempotent? isreentrant? Maybe just a vague isrepeatable?

--
Greg

On 24 Dec 2008, at 20:16, "Hitoshi Harada" <umi(dot)tanuki(at)gmail(dot)com> wrote:

> 2008/12/25 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>>> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>>>> Unless we want to move the goalposts on what an aggregate is
>>>> allowed
>>>> to do internally, we're going to have to change this to re-
>>>> aggregate
>>>> repeatedly. Neither prospect is appetizing in the least.
>>
>>> Does it currently copy the state datum before calling the final
>>> function?
>>> Would that help?
>>
>> No. The entire point of what we have now formalized as "aggregates
>> with
>> internal-type transition values" is that the transition value isn't
>> necessarily a single palloc chunk. For stuff like array_agg, the
>> performance costs of requiring that are enormous.
>>
>> On looking at what array_agg does, it seems the issue there is that
>> the final-function actually deletes the working state when it thinks
>> it's done with it (see construct_md_array). It would probably be
>> possible to fix things so that it doesn't do that when it's called by
>> a WindowAgg instead of a regular Agg. What I'm more concerned about
>> is what third-party code will break. contrib/intagg has done this
>> type of thing since forever, and I'm sure people have copied that...
>
> I have concerned it once before on the first design of the window
> functions. I don't have much idea about all over the aggregate
> functions but at least count(*) does some assumption of AggState in
> its context. So I concluded when the window functions are introduced
> it must be announced that if you use aggregate in the window context,
> you must be sure it supports window as well as aggregate. It is
> because currently (<= 8.3) aggregates are assumed it is called in
> AggState only but the assumption would be broken now. It is designed,
> and announcing helps much third party code to support or not to
> support window functions (i.e. you can stop by error if window is not
> supported by the function).
>
> Regards,
>
> --
> Hitoshi Harada

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hitoshi Harada 2008-12-25 10:49:37 Re: Window-functions patch handling of aggregates
Previous Message Jaime Casanova 2008-12-25 01:22:30 hot standby on mingw