Re: Combining Aggregates

On Wed, Jan 20, 2016 at 7:38 AM, David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> Agreed. So I've attached a version of the patch which does not have any of
> the serialise/deserialise stuff in it.

I re-reviewed this and have committed most of it with only minor
kibitizing. A few notes:

- I changed the EXPLAIN code, since it failed to display the aggregate
node's mode of operation in non-text format.

- I removed some of the planner support, since I'm not sure that it's
completely correct and I'm very sure that it contains more code
duplication than I'm comfortable with (set_combineagg_references in
particular). Please feel free to resubmit this part, perhaps in
combination with code that actually uses it for something. But please
also think about whether there's a way to reduce the duplication, and
maybe consider adding some comments, too.

- I'm not sure that you made the right call regarding reusing
transfn_oid and transfn for combine functions, vs. adding separate
fields. It is sort of logical and has the advantage of keeping the
data structure smaller, but it might also be thought confusing or
inappropriate on other grounds. But I think we can change it later if
that comes to seem like the right thing to do, so I've left it the way
you had it for now.

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