From: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila(at)enterprisedb(dot)com> |
Subject: | Re: Combining Aggregates |
Date: | 2016-03-21 18:18:35 |
Message-ID: | CAKJS1f9bq_1ysp2_+AXksrUU+-fOb8tSxN5Tczj77SaiWpFzVA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 22 March 2016 at 05:27, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sat, Mar 19, 2016 at 11:48 PM, David Rowley
> <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
>> 0002: Adds serial/de-serial function support to CREATE AGGREGATE,
>> contains minor fix-ups from last version.
>
> This looks pretty good, but don't build_aggregate_serialfn_expr and
> build_aggregate_deserialfn_expr compile down to identical machine
> code? Keeping two copies of the same code with different parameter
> names is a degree of neatnik-ism I'm not sure I can swallow.
Good point. I've removed the deserial one, and renamed the arguments
to arg_input_type and arg_output_type.
> The only caller to make_partialgroup_input_target() passes true for
> the additional argument. That doesn't seem right.
Yeah, I coded that with non-parallel use cases in mind. I very much
thing we'll end up using that flag in 9.7, but perhaps that's not a
good enough reason to be adding it in 9.6. I've pulled it out of the
patch for now, but I could also go further and pull the flag from the
whole patch, as we could just serialize the states in nodeAgg.c when
finalizeAggs == false and deserialize when combineStates == true.
> Maybe error messages should refer to "aggregate serialization
> function" and "aggregate deserialization function" instead of
> "aggregate serial function" and "aggregate de-serial function".
That's probably a good idea.
> - * Other behavior is also supported and is controlled by the
> 'combineStates'
> - * and 'finalizeAggs'. 'combineStates' controls whether the trans func or
> - * the combine func is used during aggregation. When 'combineStates' is
> - * true we expect other (previously) aggregated states as input
> rather than
> - * input tuples. This mode facilitates multiple aggregate stages which
> - * allows us to support pushing aggregation down deeper into
> the plan rather
> - * than leaving it for the final stage. For example with a query such as:
> + * Other behavior is also supported and is controlled by the
> 'combineStates',
> + * 'finalizeAggs' and 'serialStates' parameters. 'combineStates' controls
> + * whether the trans func or the combine func is used during aggregation.
> + * When 'combineStates' is true we expect other (previously) aggregated
> + * states as input rather than input tuples. This mode
> facilitates multiple
> + * aggregate stages which allows us to support pushing aggregation down
> + * deeper into the plan rather than leaving it for the final stage. For
> + * example with a query such as:
>
> I'd omit this hunk. The serialStates thing is really separate from
> what's being talked about here, and you discuss it further down.
Removed.
I've attached 2 of the patches which are affected by the changes.
Thanks for looking over this.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
0001-Allow-INTERNAL-state-aggregates-to-participate-in-pa_2016-03-22a.patch | application/octet-stream | 90.0 KB |
0002-Add-various-aggregate-combine-and-serialize-de-seria_2016-03-22a.patch | application/octet-stream | 63.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2016-03-21 18:21:38 | Re: Parallel Aggregate |
Previous Message | David G. Johnston | 2016-03-21 18:09:02 | Re: Request - repeat value of \pset title during \watch interations |