On 20 January 2016 at 10:54, Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Jan 19, 2016 at 4:50 PM, David Rowley > <david.row...@2ndquadrant.com> wrote: > >> Oh, one more point: is there any reason why all of this needs to be a > >> single (giant) patch? I feel like the handling of internal states > >> could be a separate patch from the basic patch to allow partial > >> aggregation and aggregate-combining, and that the latter could be > >> committed first if we had a reasonably final version of it. That > >> seems like it would be easier from a review standpoint, and might > >> allow more development to proceed in parallel, too. > > > > I didn't ever really imagine that I'd include any actual new combinefns > or > > serialfn/deserialfn in this patch. One set has of course now ended up in > > there, I can move these off to the test patch for now. > > > > Did you imagine that the first patch in the series would only add the > > aggcombinefn column to pg_aggregate and leave the aggserialfn stuff until > > later? > > I think that would be a sensible way to proceed. > > > I thought it seemed better to get the infrastructure committed in one > > hit, then add a bunch of new combinefn, serialfn, deserialfns for > existing > > aggregates in follow-on commits. > > To my mind, priority #1 ought to be putting this fine new > functionality to some use. Expanding it to every aggregate we've got > seems like a distinctly second priority. That's not to say that it's > absolutely gotta go down that way, but those would be my priorities. Agreed. So I've attached a version of the patch which does not have any of the serialise/deserialise stuff in it. I've also attached a test patch which modifies the grouping planner to add a Partial Aggregate node, and a final aggregate node when it's possible. Running the regression tests with this patch only shows up variances in the EXPLAIN outputs, which is of course expected. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
combine_aggregate_state_d6d480b_2016-01-21.patch
Description: Binary data
combine_aggs_test_v6.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
