On Sat, Mar 19, 2016 at 11:48 PM, David Rowley <david.row...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
