Re: Combining Aggregates

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(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 18:33:50
Message-ID: CA+TgmoZcPD_pNXSqm+cCv14dBAM+pTnkRLeAPnnJ6o2DGxgqvQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 5, 2016 at 10:19 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I'm going to concede the point that this shouldn't really be a
> priority for 9.6, but I might want to come back to it later.

It seems to me that if two aggregates are using the same transition
function, they ought to also be using the same combine, serialization,
and deserialization functions. I wrote a query to find cases where
that wasn't so and found a few, which I changed before committing.

DATA(insert ( 2100 n 0 int8_avg_accum numeric_poly_avg
int8_avg_combine int8_avg_serialize int8_avg_deserialize
int8_avg_accum int8_avg_accum_inv numeric_poly_avg f f 0 2281
17 48 2281 48 _null_ _null_ ));
DATA(insert ( 2107 n 0 int8_avg_accum numeric_poly_sum
numeric_poly_combine int8_avg_serialize int8_avg_deserialize
int8_avg_accum int8_avg_accum_inv numeric_poly_sum f f 0 2281
17 48 2281 48 _null_ _null_ ));

I changed the second of these from numeric_poly_combine to int8_avg_combine.

DATA(insert ( 2103 n 0 numeric_avg_accum numeric_avg
numeric_avg_combine numeric_avg_serialize numeric_avg_deserialize
numeric_avg_accum numeric_accum_inv numeric_avg f f 0 2281
17 128 2281 128 _null_ _null_ ));
DATA(insert ( 2114 n 0 numeric_avg_accum numeric_sum
numeric_combine numeric_avg_serialize
numeric_avg_deserialize numeric_avg_accum numeric_accum_inv
numeric_sum f f 0 2281 17 128 2281 128 _null_ _null_
));

I changed the second of these from numeric_combine to
numeric_avg_combine. I also added a regression test for this. Please
let me know if I'm off-base here.

Committed 0002+0003 with those changes, some minor cosmetic stuff, and
of course the obligatory catversion bump. Oh, and fixed an OID
conflict with the patch Magnus just committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-04-05 18:37:25 Re: Move PinBuffer and UnpinBuffer to atomics
Previous Message Julien Rouhaud 2016-04-05 18:25:45 Re: Choosing parallel_degree