Re: Parallel Aggregates for string_agg and array_agg

On 27 March 2018 at 13:45, David Rowley <david.rowley@2ndquadrant.com> wrote:
> On 27 March 2018 at 12:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Oh, I thought of another thing that would need to get done, if we decide
>> to commit this.  array_agg_serialize/deserialize only work if the array
>> element type has send/receive functions.  The planner's logic to decide
>> whether partial aggregation is possible doesn't look any deeper than
>> whether the aggregate has serialize/deserialize, but I think we have to
>> teach it that if it's these particular serialize/deserialize functions,
>> it needs to check the array element type.  Otherwise we'll get runtime
>> failures if partial aggregation is attempted on array_agg().
>>
>> This would not matter for any core datatypes, since they all have
>> send/receive functions, but I imagine it's still pretty common for
>> extension types not to.
>>
>> For my money it'd be sufficient to hard-code the test like
>>
>>     if ((aggserialfn == F_ARRAY_AGG_SERIALIZE ||
>>          aggdeserialfn == F_ARRAY_AGG_DESERIALIZE) &&
>>         !array_element_type_has_send_and_recv(exprType(aggregate input)))
>
> I'd failed to consider this.
>
> Would it not be cleaner to just reject trans types without a
> send/receive function? Or do you think that would exclude other valid
> cases?

Seems I didn't mean "trans types". I should have said aggregate
function argument types.

The attached I adds this check.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services