Re: Combining Aggregates

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila(at)enterprisedb(dot)com>
Subject: Re: Combining Aggregates
Date: 2016-04-05 07:55:04
Message-ID: CAKJS1f-mDB-iZt0p7xafg6CWLX_0REnmrsdF0fTc29gmP_JWaw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 5 April 2016 at 16:54, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Mar 30, 2016 at 3:34 PM, David Rowley
> <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
>> I wrote 0002 - 0004, these were reviewed by Tomas.
>> 0005 is Haribabu's patch: Reviewed by Tomas and I.
>
> I think it might be a good idea if these patches made less use of
> bytea and exposed the numeric transition values as, say, a 2-element
> array of numeric.

Well, if you have a look at NumericAggState you can see it's not quite
as simple as an array of numeric, unless of course you'd be willing to
spend the extra cycles, use the extra memory, and bandwidth to convert
those int64's to numeric too, then it could be made to work. To do are
you describe properly, we'd need a composite type.

> Maybe this doesn't really matter, but it's not
> impossible that these values could be exposed somewhere, and a numeric
> is an awful lot more user-friendly than an essentially opaque bytea.

hmm, isn't that why we have a deserialisation functions? Do you see a
use case where these won't be available?

> One of the things I eventually want to figure out how to do with this
> is distributed aggregation across multiple shards, and I think it
> might be better to have the value being sent over the wire be
> something like {26.6,435.12} rather than \x1234...
>
> Thoughts?

I've not yet read the design spec for sharding in Postgres. If there's
something I can read over to get an idea of why you think this won't
work, then maybe we can come to a good conclusion that way. But if I
take a guess, then I'd have imagined that we'd not support sharding
over different major versions, and if we really needed to change the
serialisation format later, then we could do so. We could even put a
note in the documents that the serialisation format may change between
major versions.

To be really honest, I'm quite worried that if I go and make this
change then my time might be wasted as I really think making that
change this late in the day is just setting this up for failure. I
really don't think we can bat this patch over the Pacific Ocean too
many times before we find ourselves inside the feature freeze. Of
course, if you really think it's no good, that's different, it can't
be committed, but "I think it might be better" does not seem like a
solid enough argument for me to want to risk trying this and delaying
further for that.

But either way, we should come try to come to a consensus quickly. I
don't want to alienate you because you think I don't want to listen to
you. We need to act quickly or we'll only have a handful of the useful
aggregates working in parallel for 9.6. I'd rather we had them all. I
hope you do too.

Thanks for looking over the patch.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2016-04-05 07:58:46 Re: Support for N synchronous standby servers - take 2
Previous Message Craig Ringer 2016-04-05 07:51:00 Re: Timeline following for logical slots