Re: Parallel Aggregates for string_agg and array_agg - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: Parallel Aggregates for string_agg and array_agg
Date
Msg-id CALNJ-vRr2jNFTjgZu3bbmsR5k3aQsVPvp6tQDo=QvJTqrTKzpQ@mail.gmail.com
Whole thread Raw
In response to Re: Parallel Aggregates for string_agg and array_agg  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: Parallel Aggregates for string_agg and array_agg
List pgsql-hackers


On Tue, Aug 2, 2022 at 4:46 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Wed, 20 Jun 2018 at 19:08, David Rowley <david.rowley@2ndquadrant.com> wrote:
> I'll submit it again when there more consensus that we want this.

Waking up this old thread again. If you don't have a copy, the entire
thread is in [1].

The remaining item that seemed to cause this patch to be rejected was
raised in [2]. The summary of that was that it might not be a good
idea to allow parallel aggregation of string_agg() and array_agg() as
there might be some people who rely on the current ordering they get
without having an ORDER BY clause in the aggregate function call.  Tom
mentioned in [3] that users might not want to add an ORDER BY to their
aggregate function because the performance of it is terrible.  That
was true up until 1349d2790 [4], where I changed how ORDER BY /
DISTINCT aggregation worked to allow the planner to provide pre-sorted
input rather than always having nodeAgg.c do the sorting.  I think
this removes quite a lot of the argument against the patch, but not
all of it.  So here goes testing the water on seeing if any opinions
have changed over the past few years?

A rebased patch is attached.

David

[1] https://www.postgresql.org/message-id/flat/CAKJS1f98yPkRMsE0JnDh72%3DAQEUuE3atiCJtPVCtjhFwzCRJHQ%40mail.gmail.com#8bbce15b9279d2da2da99071f732a99d
[2] https://www.postgresql.org/message-id/6538.1522096067@sss.pgh.pa.us
[3] https://www.postgresql.org/message-id/18594.1522099194@sss.pgh.pa.us
[4] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=1349d2790bf48a4de072931c722f39337e72055e
Hi,
For array_agg_combine():

+       if (state1->alen < reqsize)
+       {
+           /* Use a power of 2 size rather than allocating just reqsize */
+           state1->alen = pg_nextpower2_32(reqsize);
...
+       state1->nelems = reqsize;

I wonder why pg_nextpower2_32(reqsize) is used in the if block. It seems reqsize should suffice.

Cheers

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Cleaning up historical portability baggage
Next
From: Julien Rouhaud
Date:
Subject: Re: [PATCH] CF app: add "Returned: Needs more interest"