Subject: Re: Parallel Aggregates for string_agg and array_agg
To: Tom Lane <tgl@sss.pgh.pa.us>, David Rowley <david.rowley@2ndquadrant.com>
Cc: Stephen Frost <sfrost@snowman.net>, Andres Freund <andres@anarazel.de>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
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>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Message-ID: <cd620e7f-a9dd-ad15-72a8-6c0ac10ea4c3@2ndquadrant.com>
Date: Mon, 26 Mar 2018 22:55:22 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <6538.1522096067@sss.pgh.pa.us>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/26/2018 10:27 PM, Tom Lane wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> [ combinefn_for_string_and_array_aggs_v7.patch ]
> 
> I spent a fair amount of time hacking on this with intent to commit,
> but just as I was getting to code that I liked, I started to have second
> thoughts about whether this is a good idea at all.  I quote from the fine
> manual:
> 
>     The aggregate functions array_agg, json_agg, jsonb_agg,
>     json_object_agg, jsonb_object_agg, string_agg, and xmlagg, as well as
>     similar user-defined aggregate functions, produce meaningfully
>     different result values depending on the order of the input
>     values. This ordering is unspecified by default, but can be controlled
>     by writing an ORDER BY clause within the aggregate call, as shown in
>     Section 4.2.7. Alternatively, supplying the input values from a sorted
>     subquery will usually work ...
> 
> I do not think it is accidental that these aggregates are exactly the ones
> that do not have parallelism support today.  Rather, that's because you
> just about always have an interest in the order in which the inputs get
> aggregated, which is something that parallel aggregation cannot support.
> 

I don't think that's quite true. I know plenty of people who do things
like this:

    SELECT
       a,
       b,
       avg(c),
       sum(d),
       array_agg(e),
       array_agg(f),
       string_agg(g)
    FROM hugetable GROUP BY a,b HAVING avg(c) > 100.89;

and then do some additional processing on the result in some way
(subquery, matview, ...). They don't really care about ordering of
values in the arrays, as long as orderings of all the arrays match.

Currently queries like this can use parallelism at all, and the patch
fixes that I believe.

> I fear that what will happen, if we commit this, is that something like
> 0.01% of the users of array_agg and string_agg will be pleased, another
> maybe 20% will be unaffected because they wrote ORDER BY which prevents
> parallel aggregation, and the remaining 80% will scream because we broke
> their queries.  Telling them they should've written ORDER BY isn't going
> to cut it, IMO, when the benefit of that breakage will accrue only to some
> very tiny fraction of use-cases.
> 

Isn't the ordering unreliable *already*? It depends on ordering of
tuples on the input. So if the table is scanned by index scan or
sequential scan, that will affect the array_agg/string_agg results. If
the input is a join, it's even more volatile.

IMHO it's not like we're making the ordering unpredictable - it's been
like that since forever.

Also, how is this different from ORDER BY clause? If a user does not
specify an ORDER BY clause, I don't think we'd care very much about
changes to output ordering due to plan changes, for example.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services