On Wed, 27 Mar 2024 at 03:00, Alexander Lakhin wrote:
> I've discovered that the test query:
> -- Ensure parallel aggregation is actually being used.
> explain (costs off) select * from v_pagg_test order by y;
>
> added by 16fd03e95 fails sometimes. For instance:
> https://buildfarm.postgresql.org
Hello David,
23.01.2023 07:37, David Rowley wrote:
I've now pushed this.
I've discovered that the test query:
-- Ensure parallel aggregation is actually being used.
explain (costs off) select * from v_pagg_test order by y;
added by 16fd03e95 fails sometimes. For instance:
https://buildfarm.p
On Thu, 19 Jan 2023 at 20:44, David Rowley wrote:
> Thanks. Pending one last look, I'm planning to move ahead with it
> unless there are any further objections or concerns.
I've now pushed this.
Thank you to everyone who reviewed or gave input on this patch.
David
Hi
út 27. 12. 2022 v 6:26 odesílatel David Rowley
napsal:
> On Wed, 20 Jun 2018 at 19:08, David Rowley
> 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 i
On Wed, 3 Aug 2022 at 14:40, Zhihong Yu wrote:
> 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 = reqsi
On Tue, Aug 2, 2022 at 4:46 PM David Rowley wrote:
> On Wed, 20 Jun 2018 at 19:08, David Rowley
> 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
On Wed, 3 Aug 2022 at 12:18, Tom Lane wrote:
> Now as against that, if the underlying relation scan is parallelized
> then you already have unpredictable input ordering and thus unpredictable
> aggregate results. So anyone who cares about that has already had
> to either disable parallelism or in
David Rowley writes:
> 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].
Hmm. My estimate of the percentages of users who will be pleased or
not hasn't really changed
On Wed, 20 Jun 2018 at 19:08, David Rowley 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 s
On 2 May 2018 at 10:14, David Rowley wrote:
> On 2 May 2018 at 08:59, Tom Lane wrote:
>> 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 proba
On 2 May 2018 at 08:59, Tom Lane wrote:
> Robert Haas writes:
>> On Mon, Mar 26, 2018 at 4:27 PM, Tom Lane 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 unaffect
On 2018-05-01 14:35:46 -0700, Mark Dilger wrote:
>
> > On May 1, 2018, at 2:11 PM, Andres Freund wrote:
> >
> > Hi,
> >
> > On 2018-05-01 14:09:39 -0700, Mark Dilger wrote:
> >> I don't care which order the data is in, as long as x[i] and y[i] are
> >> matched correctly. It sounds like this pa
> On May 1, 2018, at 2:11 PM, Andres Freund wrote:
>
> Hi,
>
> On 2018-05-01 14:09:39 -0700, Mark Dilger wrote:
>> I don't care which order the data is in, as long as x[i] and y[i] are
>> matched correctly. It sounds like this patch would force me to write
>> that as, for example:
>>
>> selec
On 2018-05-01 17:16:16 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2018-05-01 14:09:39 -0700, Mark Dilger wrote:
> >> I don't care which order the data is in, as long as x[i] and y[i] are
> >> matched correctly. It sounds like this patch would force me to write
> >> that as, for example
Andres Freund writes:
> On 2018-05-01 14:09:39 -0700, Mark Dilger wrote:
>> I don't care which order the data is in, as long as x[i] and y[i] are
>> matched correctly. It sounds like this patch would force me to write
>> that as, for example:
>>
>> select array_agg(a order by a, b) AS x, array_a
Hi,
On 2018-05-01 14:09:39 -0700, Mark Dilger wrote:
> I don't care which order the data is in, as long as x[i] and y[i] are
> matched correctly. It sounds like this patch would force me to write
> that as, for example:
>
> select array_agg(a order by a, b) AS x, array_agg(b order by a, b) AS y
> On Mar 27, 2018, at 7:58 AM, Tom Lane wrote:
>
> David Rowley writes:
>> On 27 March 2018 at 13:26, Alvaro Herrera wrote:
>>> synchronized_seqscans is another piece of precedent in the area, FWIW.
>
>> This is true. I guess the order of aggregation could be made more
>> certain if we remove
Robert Haas writes:
> On Mon, Mar 26, 2018 at 4:27 PM, Tom Lane 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
>
On Mon, Mar 26, 2018 at 4:27 PM, Tom Lane wrote:
> 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 s
Thanks for your input on all this. It's good to some words from people
using the software rather than just writing it.
On 6 April 2018 at 07:10, Tels wrote:
> PS: We use string_agg() in a case where we first agg each row, then
> string_agg() all rows, and the resulting string is really huge. We d
On 04/05/2018 05:41 AM, David Rowley wrote:
> Hi Tomas,
>
> Thanks for taking another look.
>
> On 5 April 2018 at 07:12, Tomas Vondra wrote:
>> Seems fine to me, although we should handle the anyarray case too, I
>> guess. That is, get_agg_clause_costs_walker() should do this too:
>>
>> /
On 04/05/2018 09:10 PM, Tels wrote:
> Moin,
>
> On Wed, April 4, 2018 11:41 pm, David Rowley wrote:
>> Hi Tomas,
>>
>> Thanks for taking another look.
>>
>> On 5 April 2018 at 07:12, Tomas Vondra
>> wrote:
>>> Other than that, the patch seems fine to me, and it's already marked as
>>> RFC so I'
Moin,
On Wed, April 4, 2018 11:41 pm, David Rowley wrote:
> Hi Tomas,
>
> Thanks for taking another look.
>
> On 5 April 2018 at 07:12, Tomas Vondra
> wrote:
>> Other than that, the patch seems fine to me, and it's already marked as
>> RFC so I'll leave it at that.
>
> Thanks.
I have one more co
Hi Tomas,
Thanks for taking another look.
On 5 April 2018 at 07:12, Tomas Vondra wrote:
> Seems fine to me, although we should handle the anyarray case too, I
> guess. That is, get_agg_clause_costs_walker() should do this too:
>
> /* Same thing for array_agg_array_(de)serialize. */
> if
On 03/31/2018 04:42 PM, David Rowley wrote:
> On 30 March 2018 at 02:55, Tomas Vondra wrote:
>> On 03/29/2018 03:09 PM, David Rowley wrote:
>>> I meant to mention earlier that I coded
>>> agg_args_have_sendreceive_funcs() to only check for send/receive
>>> functions. Really we could allow a byva
On 30 March 2018 at 02:55, Tomas Vondra wrote:
> On 03/29/2018 03:09 PM, David Rowley wrote:
>> I meant to mention earlier that I coded
>> agg_args_have_sendreceive_funcs() to only check for send/receive
>> functions. Really we could allow a byval types without send/receive
>> functions, since the
On 03/29/2018 03:09 PM, David Rowley wrote:
> On 30 March 2018 at 02:00, Tomas Vondra wrote:
>> On 03/29/2018 05:49 AM, David Rowley wrote:
>>> Attached is v9, which is based on Tom's v8 but includes the new tests
>>> and I think the required fix to disable use of the serial/deserial
>>> functio
On 30 March 2018 at 02:00, Tomas Vondra wrote:
> On 03/29/2018 05:49 AM, David Rowley wrote:
>> Attached is v9, which is based on Tom's v8 but includes the new tests
>> and I think the required fix to disable use of the serial/deserial
>> function for array_agg().
>>
>
> I have only looked at the
On 03/29/2018 05:49 AM, David Rowley wrote:
> On 29 March 2018 at 03:05, Tomas Vondra wrote:
>> On 03/28/2018 03:54 PM, Tom Lane wrote:
>>> I had in mind to look at exprType() of the argument.
>>
>> Right, I'm fine with that.
>
> Attached is v9, which is based on Tom's v8 but includes the new t
On 29 March 2018 at 03:05, Tomas Vondra wrote:
> On 03/28/2018 03:54 PM, Tom Lane wrote:
>> I had in mind to look at exprType() of the argument.
>
> Right, I'm fine with that.
Attached is v9, which is based on Tom's v8 but includes the new tests
and I think the required fix to disable use of the
On 27 March 2018 at 09:27, Tom Lane wrote:
> The main thing that remains undone is to get some test coverage ---
> AFAICS, none of these new functions get exercised in the standard
> regression tests.
I've attached a patch which implements some tests for the new functions.
--
David Rowley
On 03/28/2018 03:54 PM, Tom Lane wrote:
> Tomas Vondra writes:
>> On 03/28/2018 05:28 AM, Tom Lane wrote:
>>> Getting a solution that would work for other polymorphic serialization
>>> functions seems like a bit of a research project to me. In the meantime,
>>> I think David's right that what w
On 28 March 2018 at 03:58, Tom Lane wrote:
> David Rowley writes:
>> On 27 March 2018 at 13:26, Alvaro Herrera wrote:
>>> synchronized_seqscans is another piece of precedent in the area, FWIW.
>
>> This is true. I guess the order of aggregation could be made more
>> certain if we remove the cost
Tomas Vondra writes:
> On 03/28/2018 05:28 AM, Tom Lane wrote:
>> Getting a solution that would work for other polymorphic serialization
>> functions seems like a bit of a research project to me. In the meantime,
>> I think David's right that what we need to look at is the actual input
>> type of
On 03/28/2018 05:28 AM, Tom Lane wrote:
> Tomas Vondra writes:
>> On 03/27/2018 04:51 AM, David Rowley wrote:
>>> Seems I didn't mean "trans types". I should have said aggregate
>>> function argument types.
>
>> I'm not sure that's better than the check proposed by Tom. An argument
>> type with
Tomas Vondra writes:
> On 03/27/2018 04:51 AM, David Rowley wrote:
>> Seems I didn't mean "trans types". I should have said aggregate
>> function argument types.
> I'm not sure that's better than the check proposed by Tom. An argument
> type without send/receive function does not necessarily mean
On 03/27/2018 04:51 AM, David Rowley wrote:
> On 27 March 2018 at 13:45, David Rowley wrote:
>> On 27 March 2018 at 12:49, Tom Lane 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
>>>
On 03/27/2018 04:58 PM, Tom Lane wrote:
> David Rowley writes:
>> On 27 March 2018 at 13:26, Alvaro Herrera wrote:
>>> synchronized_seqscans is another piece of precedent in the area, FWIW.
>
>> This is true. I guess the order of aggregation could be made more
>> certain if we remove the cost b
David Rowley writes:
> On 27 March 2018 at 13:26, Alvaro Herrera wrote:
>> synchronized_seqscans is another piece of precedent in the area, FWIW.
> This is true. I guess the order of aggregation could be made more
> certain if we remove the cost based optimiser completely, and just
> rely on a s
On Tue, Mar 27, 2018 at 5:36 PM, Magnus Hagander wrote:
> On Tue, Mar 27, 2018 at 12:28 AM, Tom Lane wrote:
>>
>> David Rowley writes:
>> > On 27 March 2018 at 09:27, Tom Lane wrote:
>> >> I do not think it is accidental that these aggregates are exactly the
>> >> ones
>> >> that do not have pa
On Tue, Mar 27, 2018 at 12:28 AM, Tom Lane wrote:
> David Rowley writes:
> > On 27 March 2018 at 09:27, Tom Lane wrote:
> >> 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
On 27 March 2018 at 13:45, David Rowley wrote:
> On 27 March 2018 at 12:49, Tom Lane 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 plann
On 27 March 2018 at 12:49, Tom Lane wrote:
> I wrote:
>> The main thing that remains undone is to get some test coverage ---
>> AFAICS, none of these new functions get exercised in the standard
>> regression tests.
>
> Oh, I thought of another thing that would need to get done, if we decide
> to c
On 27 March 2018 at 13:26, Alvaro Herrera wrote:
> David Rowley wrote:
>
>> Anyway, the options are not zero for anyone who is strongly affected
>> with no other workaround. They just need to disable parallel query,
>> which to me seems fairly similar to the 8.4 release note's "the
>> previous beh
David Rowley wrote:
> Anyway, the options are not zero for anyone who is strongly affected
> with no other workaround. They just need to disable parallel query,
> which to me seems fairly similar to the 8.4 release note's "the
> previous behavior can be restored by disabling enable_hashagg"
synch
Hi,
On 2018-03-27 13:14:15 +1300, David Rowley wrote:
> I have to say, it really would be a shame to have this concern block
> us from future optimisations in aggregation.
Yea, I think that's an important point. By the dint of Tom's argument
we're never going to be able to provide parallelism for
On 27 March 2018 at 11:28, Tom Lane wrote:
> David Rowley writes:
>> This very much reminds me of something that exists in the 8.4 release notes:
>>> SELECT DISTINCT and UNION/INTERSECT/EXCEPT no longer always produce sorted
>>> output (Tom)
>
> That's a completely false analogy: we got a signif
I wrote:
> The main thing that remains undone is to get some test coverage ---
> AFAICS, none of these new functions get exercised in the standard
> regression tests.
Oh, I thought of another thing that would need to get done, if we decide
to commit this. array_agg_serialize/deserialize only work
David Rowley writes:
> On 27 March 2018 at 09:27, Tom Lane wrote:
>> 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
>>
On 03/26/2018 11:19 PM, Tom Lane wrote:
> Tomas Vondra writes:
>> On 03/26/2018 10:27 PM, Tom Lane 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
On 27 March 2018 at 09:27, Tom Lane wrote:
> 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 f
On 27 March 2018 at 10:20, Stephen Frost wrote:
> In the end, I do tend to agree that we probably should add parallel
> support to these aggregates, but it'd also be nice to hear from those
> who had worked to add parallelism to the various aggregates as to if
> there was some reason these were sk
Stephen Frost writes:
> Yeah, there certainly seems like a lot of opportunity for the ordering
> to end up being volatile already and queries depending on it not
> changing really shouldn't be making that assumption. I do think it was
> probably a bad move on our part to say that ordering a subqu
Tomas Vondra writes:
> On 03/26/2018 10:27 PM, Tom Lane 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
>> paralle
Greetings,
* Tomas Vondra (tomas.von...@2ndquadrant.com) wrote:
> On 03/26/2018 10:27 PM, Tom Lane 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
On 03/26/2018 10:27 PM, Tom Lane wrote:
> David Rowley 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
David Rowley 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:
On 27 March 2018 at 02:20, Stephen Frost wrote:
> Just to be clear- the segfault is just happening with your patch and
> you're just contemplating having string_agg always allocate state on the
> first call, similar to what int8_avg_accum() does?
>
> If so, then, yes, that seems alright to me.
I
Greetings,
* David Rowley (david.row...@2ndquadrant.com) wrote:
> On 26 March 2018 at 15:26, Stephen Frost wrote:
> > The header at the top of datumCopy() pretty clearly says that it's for
> > "non-NULL" datums, yet this function seems to be happily ignoring that
> > and just trying to use it to
On 26 March 2018 at 15:26, Stephen Frost wrote:
> The header at the top of datumCopy() pretty clearly says that it's for
> "non-NULL" datums, yet this function seems to be happily ignoring that
> and just trying to use it to copy everything. Perhaps I'm missing
> something, but I don't see anythi
Greetings,
* David Rowley (david.row...@2ndquadrant.com) wrote:
> On 16 March 2018 at 13:46, Tomas Vondra wrote:
> > I've done more testing on this patch, and I haven't found any new issues
> > with it, so PFA I'm marking it as ready for committer.
>
> Great! Many thanks for the review.
I read
On 16 March 2018 at 13:46, Tomas Vondra wrote:
> I've done more testing on this patch, and I haven't found any new issues
> with it, so PFA I'm marking it as ready for committer.
Great! Many thanks for the review.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Devel
On 03/11/2018 12:10 PM, Tomas Vondra wrote:
>
> ...
>
> Yeah, seems fine to me. I wonder what else would be needed before
> switching the patch to RFC. I plan to do that after a bit more
> testing sometime early next week, unless someone objects.
>
I've done more testing on this patch, and I
On 03/11/2018 07:31 AM, David Rowley wrote:
> On 11 March 2018 at 12:11, Tomas Vondra wrote:
>> On 03/05/2018 04:51 AM, David Rowley wrote:
>>> On 5 March 2018 at 04:54, Tomas Vondra wrote:
>>> Consider the following slightly backward-looking case;
>>>
>>> select string_agg(',', x::text) from g
On 11 March 2018 at 12:11, Tomas Vondra wrote:
> On 03/05/2018 04:51 AM, David Rowley wrote:
>> On 5 March 2018 at 04:54, Tomas Vondra wrote:
>> Consider the following slightly backward-looking case;
>>
>> select string_agg(',', x::text) from generate_Series(1,10)x;
>> string_agg
>> ---
On 03/05/2018 04:51 AM, David Rowley wrote:
> On 5 March 2018 at 04:54, Tomas Vondra wrote:
>> 1) There seems to be forgotten declaration of initArrayResultInternal in
>> arrayfuncs.c. I suppose you've renamed it to initArrayResultWithSize and
>> moved it to a header file, and forgotten to remov
On 5 March 2018 at 04:54, Tomas Vondra wrote:
> 1) There seems to be forgotten declaration of initArrayResultInternal in
> arrayfuncs.c. I suppose you've renamed it to initArrayResultWithSize and
> moved it to a header file, and forgotten to remove this bit.
Oops. Must be left over from trying th
Hi,
I've looked at the rebased patch you sent yesterday, and I do have a
couple of comments.
1) There seems to be forgotten declaration of initArrayResultInternal in
arrayfuncs.c. I suppose you've renamed it to initArrayResultWithSize and
moved it to a header file, and forgotten to remove this bi
I've attached a rebased patch which fixes the conflicts caused by fd1a421.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
combinefn_for_string_and_array_aggs_v4.patch
Description: Binary data
On 2018-03-02 13:48:00 +1300, David Rowley wrote:
> On 2 March 2018 at 10:26, Andres Freund wrote:
> > FWIW, I've heard numerous people yearn for json*agg
>
> I guess it'll need to be PG12 for now. I'd imagine string_agg and
> array_agg are more important ones to tick off for now, but I bet many
On 2 March 2018 at 10:26, Andres Freund wrote:
> On 2017-12-18 03:30:55 +1300, David Rowley wrote:
>> Just a handful of aggregates now don't support partial aggregation;
>>
>> postgres=# select aggfnoid from pg_aggregate where aggcombinefn=0 and
>> aggkind='n';
>> aggfnoid
>>
Thanks for looking at the patch.
On 2 March 2018 at 08:33, David Steele wrote:
> This patch applies but no longer builds:
...
> Looks like duplicate OIDs in pg_proc.h.
Who stole my OIDs?!
Updated patch attached.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Devel
Hi,
On 2017-12-18 03:30:55 +1300, David Rowley wrote:
> While working on partial aggregation a few years ago, I didn't really
> think it was worthwhile allowing partial aggregation of string_agg and
> array_agg. I soon realised that I was wrong about that and allowing
> parallelisation of these ag
Hi David,
On 12/17/17 9:30 AM, David Rowley wrote:
>
> I'm going to add this to PG11's final commitfest rather than the
> January 'fest as it seems more like a final commitfest type of patch.
This patch applies but no longer builds:
$ make -C /home/vagrant/test/build
<...>
cd /postgres/src/incl
Hi,
While working on partial aggregation a few years ago, I didn't really
think it was worthwhile allowing partial aggregation of string_agg and
array_agg. I soon realised that I was wrong about that and allowing
parallelisation of these aggregates still could be very useful when
many rows are fil
75 matches
Mail list logo
