On Wed, Jan 20, 2016 at 7:38 AM, David Rowley <david.row...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
