On 19 March 2016 at 05:46, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, Mar 16, 2016 at 5:05 PM, David Rowley > <david.row...@2ndquadrant.com> wrote: >>> Cool! Why not initialize aggpartialtype always? >> >> Because the follow-on patch sets that to either the serialtype or the >> aggtranstype, depending on if serialisation is required. Serialisation >> is required for parallel aggregate, but if we're performing the >> partial agg in the main process, then we'd not need to do that. This >> could be solved by adding more fields to AggRef to cover the >> aggserialtype and perhaps expanding aggpartial into an enum mode which >> allows NORMAL, PARTIAL, PARTIAL_SERIALIZE, and have exprType() pay >> attention to the mode and return 1 of the 3 possible types. > > Urk. That might still be better than what you have right now, but > it's obviously not great. How about ditching aggpartialtype and > adding aggoutputtype instead? Then you can always initialize that to > whatever it's supposed to be based on the type of aggregation you are > doing, and exprType() can simply return that field.
hmm, that might be better, but it kinda leaves aggpartial without much of a job to do. The only code which depends on that is the sanity checks that I added in execQual.c, and it does not really seem worth keeping it for that. The only sanity check that I can think to do here is if (aggstate->finalizeAggs && aggref->aggoutputtype != aggref->aggtype) -- we have a problem. Obviously we can't check that for non-finalize nodes since the aggtype can match the aggoutputtype for legitimate reasons. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
