From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: David Rowley <david.rowley@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Parallel Aggregates for string_agg and array_agg
Date: Wed, 28 Mar 2018 02:06:37 +0200
Message-ID: <452e8b7f-418b-4e61-c212-c0e4410b94ad@2ndquadrant.com> (raw)
In-Reply-To: <CAKJS1f9oTWG4oc7B2WfDhT11KhxeV1miLQ+PzB6hY=iJeHM28A@mail.gmail.com>
References: <CAKJS1f8cB3cc5AK=wPvByhU_jKkgd81iRt2kE6cru8uxx2HCuA@mail.gmail.com>
<c302afa6-6415-27a5-336a-e1d4332fab09@2ndquadrant.com>
<CAKJS1f9xEZdTaBbBEKY+TCKEoSZYaH1oSAtsEJHSYDJCCNnQPQ@mail.gmail.com>
<24abe018-b3c6-2117-b5b9-39e660db00bd@2ndquadrant.com>
<CAKJS1f8LV7AT=AAhdYGKtGrGkSkEgO6C_SW2Ztz1sR3encisqw@mail.gmail.com>
<1b6ee15c-c218-761e-0120-c1ed438e2db2@2ndquadrant.com>
<657be73a-a21d-0ae1-5c62-870afbdfddd4@2ndquadrant.com>
<CAKJS1f-bKAD+s-CJetRQPjmrR-gF-kfvwfzhQm-A0PA-nsr1xw@mail.gmail.com>
<20180326022647.GL24540@tamriel.snowman.net>
<CAKJS1f-axkksX3XzduLJuTL=o8CSwf1NaQYO6=iHSXR_B3pCKQ@mail.gmail.com>
<20180326132009.GN24540@tamriel.snowman.net>
<CAKJS1f8JVpUUkYP2c+RyVRdpS1dnjB_DA5fbcDSL3qfMRqc81Q@mail.gmail.com>
<6538.1522096067@sss.pgh.pa.us>
<25812.1522108164@sss.pgh.pa.us>
<CAKJS1f87rFqL0=ksfs+3mmQvg_R0RRGEXyayxHkwYnFqeKpngQ@mail.gmail.com>
<CAKJS1f9oTWG4oc7B2WfDhT11KhxeV1miLQ+PzB6hY=iJeHM28A@mail.gmail.com>
On 03/27/2018 04:51 AM, David Rowley wrote:
> 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.
>
I'm not sure that's better than the check proposed by Tom. An argument
type without send/receive function does not necessarily mean we can't
serialize/deserialize the trans value. Because who says the argument
value will be embedded in the trans value?
For example, I might create an aggregate to build a bloom filter,
accepting anyelement. That will hash the argument value, and obviously
has no issues with serializing/deserializing the trans value.
This check would effectively disable parallelism for all aggregates with
anyarray, anyelement, anynonarray, anyenum (and a few more) arguments.
That for example includes jsonb_agg, which some people already mentioned
as another candidate for enabling parallelism. Not great, I guess.
Tom's solution is much more focused - it recognizes that array_agg is a
fairly rare case where the (actual) argument type gets stored in the
trans type, and detects that.
PS: The function is misnamed - you're talking about argument types, yet
the function name refers to transtypes.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, david.rowley@2ndquadrant.com, tgl@sss.pgh.pa.us, sfrost@snowman.net, andres@anarazel.de
Subject: Re: Parallel Aggregates for string_agg and array_agg
In-Reply-To: <452e8b7f-418b-4e61-c212-c0e4410b94ad@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox