Re: Combining Aggregates

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.

The only caller to make_partialgroup_input_target() passes true for
the additional argument. That doesn't seem right.

Maybe error messages should refer to "aggregate serialization
function" and "aggregate deserialization function" instead of
"aggregate serial function" and "aggregate de-serial function".

- * 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.

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