On 19 March 2016 at 05:46, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Mar 16, 2016 at 5:05 PM, David Rowley
> <david.rowley@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