On Wed, Mar 16, 2016 at 6:49 AM, David Rowley <david.row...@2ndquadrant.com> wrote: > On 16 March 2016 at 15:04, Robert Haas <robertmh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
