Re: Parallel Aggregates for string_agg and array_agg
David Rowley <david.rowley@2ndquadrant.com>
From: David Rowley <david.rowley@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Stephen Frost <sfrost@snowman.net>, Tomas Vondra <tomas.vondra@2ndquadrant.com>,
Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2018-05-01T22:14:25Z
Lists: pgsql-hackers
On 2 May 2018 at 08:59, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Mon, Mar 26, 2018 at 4:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> 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. > >> I think your estimated percentages here are wildly inaccurate. > > My estimate for the number of people positively impacted could be off > by a factor of a thousand, and it still wouldn't change the conclusion > that this will hurt more people than it helps. It's probably best to have this argument again in 6 months or so. Someone might decide to put some effort into teaching the planner about ordered aggregates so it can choose to provide pre-sorted input to aggregate functions so that nodeAgg.c no longer has to perform the costly explicit sorts. Windowing functions already do this for the first window clause, so it might not be too hard to do it for the first aggregate, and any subsequent aggregates which share the same ORDER BY / DISTINCT clause. We'd probably need to add some sort of evaluation order to the Aggref, but ... details need not be discussed here ... I think if we had that, then the current objection to the patch loses quite a bit of traction. I imagine Tom will give up his objection given that he wrote this: On 27 March 2018 at 11:28, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Maybe what that says is that rather than giving up on this altogether, > we should shelve it till we have less stupid planner+executor behavior > for ORDER BY inside aggregates. That's been on the to-do list for > a long while, but not risen to the top ... So it seems there's probably room here to please everyone. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Commits
-
Fix unstable aggregate regression test
- f5d0e866404a 16.3 landed
- d6a6957d53e3 17.0 landed
-
Allow parallel aggregate on string_agg and array_agg
- 16fd03e95654 16.0 landed
-
Improve performance of ORDER BY / DISTINCT aggregates
- 1349d2790bf4 16.0 cited
-
Support ORDER BY within aggregate function calls, at long last providing a
- 34d26872ed81 9.0.0 cited
-
Fix broken markup.
- 6a6efb964092 9.0.0 cited