pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: David Rowley <david.rowley@2ndquadrant.com>
Cc: 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: Thu, 5 Apr 2018 22:49:03 +0200
Message-ID: <8f76820a-5d3a-bade-59c3-a3852b730826@2ndquadrant.com> (raw)
In-Reply-To: <CAKJS1f_yDsOrSffHBqAN3Gx27R1vW6OHaKy_EwnhnLZJmo8+Mw@mail.gmail.com>
References: <CAKJS1f8cB3cc5AK=wPvByhU_jKkgd81iRt2kE6cru8uxx2HCuA@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>
	<19216.1522207699@sss.pgh.pa.us>
	<18741.1522245278@sss.pgh.pa.us>
	<1ffa699d-ff84-4c21-10cb-4d8b36017bab@2ndquadrant.com>
	<CAKJS1f95uF3h0PdMj-zqnTcJXnF68bT4nUKgiXtAdEz44W5HWg@mail.gmail.com>
	<13e3fd13-8152-72be-4e3c-8095e6ec0995@2ndquadrant.com>
	<CAKJS1f-S493AXpKAiri7cSFfhy2vy5kLHzkTDXC5QzoxMJ5Lkg@mail.gmail.com>
	<73ea70fa-9ada-deb5-549e-02fa2f0969c6@2ndquadrant.com>
	<CAKJS1f9jP_b=yoLa5bt9WPC5mtNgMKFgP6FH2ejZ_PqYyxbDAQ@mail.gmail.com>
	<ad2283b9-64bd-3016-da60-648ba9c13c5f@2ndquadrant.com>
	<CAKJS1f_yDsOrSffHBqAN3Gx27R1vW6OHaKy_EwnhnLZJmo8+Mw@mail.gmail.com>



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 <tomas.vondra@2ndquadrant.com> 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 ((aggserialfn == F_ARRAY_AGG_ARRAY_SERIALIZE ||
>>          aggdeserialfn == F_ARRAY_AGG_ARRAY_DESERIALIZE) &&
>>          !agg_args_support_sendreceive(aggref))
>>         costs->hasNonSerial = true;
> 
> hmm, array_agg_array_serialize and array_agg_array_deserialize don't
> use the send/receive functions though, so not sure why that's
> required?
> 
>> Other than that, the patch seems fine to me, and it's already marked as
>> RFC so I'll leave it at that.
> 
> Thanks.
> 
>> The last obstacle seems to be the argument about the risks of the patch
>> breaking queries of people relying on the ordering. Not sure what's are
>> the right next steps in this regard ...
> 
> yeah, seems like a bit of a stalemate.
> 
> Personally, think if the group of people Tom mentions do exist, then
> they've already been through some troubled times since Parallel Query
> was born. I'd hope they've already put up their defenses due to the
> advent of that feature. I stand by my thoughts that it's pretty
> bizarre to draw the line here when we've probably been causing these
> people issues for many years already. I said my piece on this already
> so likely not much point in going on about it. These people are also
> perfectly capable of sidestepping the whole issue with setting
> max_parallel_workers_per_gather TO 0.
> 
> Perhaps one solution is to drop the string_agg and keep the array_agg.
> Unsure if that would be good enough for Tom? More people seem to have
> voiced that array_agg ordering is generally less of a concern, which I
> imagine is probably true, but my opinion might not matter here as I'm
> the sort of person who if I needed a specific ordering I'd have
> written an ORDER BY clause.
> 

I don't really see how dropping string_agg would solve anything. Either
the risks apply to both these functions, or neither of them I think. If
anything, doing this only for one of them would be inconsistent and
surprising, considering how similar they are.

regards

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




view thread (77+ messages)  latest in thread

Message-ID: <8f76820a-5d3a-bade-59c3-a3852b730826@2ndquadrant.com>
Permalink:  ../8f76820a-5d3a-bade-59c3-a3852b730826@2ndquadrant.com/
Also on:    postgresql.org/message-id/8f76820a-5d3a-bade-59c3-a3852b730826@2ndquadrant.com

 · 

reply
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: <8f76820a-5d3a-bade-59c3-a3852b730826@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