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: Wed, 4 Apr 2018 21:12:54 +0200
Message-ID: <ad2283b9-64bd-3016-da60-648ba9c13c5f@2ndquadrant.com> (raw)
In-Reply-To: <CAKJS1f9jP_b=yoLa5bt9WPC5mtNgMKFgP6FH2ejZ_PqYyxbDAQ@mail.gmail.com>
References: <CAKJS1f8cB3cc5AK=wPvByhU_jKkgd81iRt2kE6cru8uxx2HCuA@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>
	<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>



On 03/31/2018 04:42 PM, David Rowley wrote:
> On 30 March 2018 at 02:55, Tomas Vondra <tomas.vondra@2ndquadrant.com> 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 serial/deserial just send the raw datums in that
>>> case, but then the function becomes
>>> agg_byref_args_have_sendreceive_funcs(), which seemed a bit obscure,
>>> so I didn't do that.  Maybe I should?
>>
>> I'd do that. Not sure the function name needs to change, but perhaps
>> agg_args_support_sendreceive() would be better - it covers both byref
>> types (which require send/receive functions) and byval (which don't).
> 
> The attached patch implements this.
> 

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;

Other than that, the patch seems fine to me, and it's already marked as
RFC so I'll leave it at that.

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 ...

regards

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




view thread (77+ messages)  latest in thread

Message-ID: <ad2283b9-64bd-3016-da60-648ba9c13c5f@2ndquadrant.com>
Permalink:  ../ad2283b9-64bd-3016-da60-648ba9c13c5f@2ndquadrant.com/
Also on:    postgresql.org/message-id/ad2283b9-64bd-3016-da60-648ba9c13c5f@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: <ad2283b9-64bd-3016-da60-648ba9c13c5f@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