Re: Parallel Aggregates for string_agg and array_agg

On 27 March 2018 at 13:45, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 27 March 2018 at 12:49, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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