Re: Parallel Aggregate

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Paul Ramsey <pramsey(at)cleverelephant(dot)ca>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Parallel Aggregate
Date: 2016-03-16 11:57:26
Message-ID: CA+Tgmoba3mWhNHmO+a8fax3_iUA1kQHxC6TXx=RYW0vQeSPTmg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 16, 2016 at 6:49 AM, David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 16 March 2016 at 15:04, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> I don't think I'd be objecting if you made PartialAggref a real
>> alternative to Aggref. But that's not what you've got here. A
>> PartialAggref is just a wrapper around an underlying Aggref that
>> changes the interpretation of it - and I think that's not a good idea.
>> If you want to have Aggref and PartialAggref as truly parallel node
>> types, that seems cool, and possibly better than what you've got here
>> now. Alternatively, Aggref can do everything. But I don't think we
>> should go with this wrapper concept.
>
> Ok, I've now gotten rid of the PartialAggref node, and I'm actually
> quite happy with how it turned out. I made
> search_indexed_tlist_for_partial_aggref() to follow-on the series of
> other search_indexed_tlist_for_* functions and have made it behave the
> same way, by returning the newly created Var instead of doing that in
> fix_combine_agg_expr_mutator(), as the last version did.
>
> Thanks for the suggestion.
>
> New patch attached.

Cool! Why not initialize aggpartialtype always?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stas Kelvich 2016-03-16 11:59:35 Re: eXtensible Transaction Manager API (v2)
Previous Message Grzegorz Sampolski 2016-03-16 11:46:54 Re: pam auth - add rhost item