From: | Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, David Rowley <dgrowleyml(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: | 2015-12-24 02:30:01 |
Message-ID: | CAJrrPGfg_deYkPX4=7s7CqCXG-6rz7oAo9_xA_6tJr4ss79yHg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Dec 24, 2015 at 1:12 PM, David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 24 December 2015 at 13:55, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
> wrote:
>>
>> On Wed, Dec 23, 2015 at 7:50 PM, David Rowley
>> <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
>> > One other part that I'm not too sure on how to deal with is how to set
>> > the
>> > data type for the Aggrefs when we're not performing finalization on the
>> > aggregate node. The return type for the Aggref in this case will be
>> > either
>> > the transtype, or the serialtype, depending on if we're serializing the
>> > states or not. To do this, I've so far just come up with
>> > set_partialagg_aggref_types() which is called during setrefs. The only
>> > other
>> > time that I can think to do this return type update would be when
>> > building
>> > the partial agg node's target list. I'm open to better ideas on this
>> > part.
>>
>>
>> Thanks for the patch. I am not sure about the proper place of this change,
>> but changing it with transtype will make all float4 and float8 aggregates
>> to
>> work in parallel. Most of these aggregates return type is typbyval and
>> transition type is a variable length.
>>
>> we may need to write better combine functions for these types to avoid
>> wrong
>> results because of parallel.
>
>
> I might be misunderstanding you here, but yeah, well, if by "write better"
> you mean "write some", then yeah :) I only touched sum(), min() and max()
> so far as I didn't need to do anything special with these. I'm not quite
> sure what you mean with the "wrong results" part. Could you explain more?
sorry for not providing clear information. To check the change of replacing
aggtype with aggtranstype is working fine or not for float8 avg. I just tried
adding a float8_combine_accum function for float8 avg similar to
float8_acuum with a difference of adding the two transvalues instead
of newval to existing transval in float8_accum function as follows,
N += transvalues1[0];
sumX += transvalues1[1];
sumX2 += transvalues1[2];
But the result came different compared to normal aggregate. The data in the
column is 1.1
postgres=# select avg(f3) from tbl where f1 < 1001 group by f3;
avg
-----------------------
2.16921537594434e-316
(1 row)
postgres=# set enable_parallelagg = false;
SET
postgres=# select avg(f3) from tbl where f1 < 1001 group by f3;
avg
------------------
1.10000000000001
(1 row)
First i thought it is because of combine function problem, but it seems
some where else the problem is present in parallel aggregate code. sorry
for the noise.
Regards,
Hari Babu
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2015-12-24 02:32:52 | Re: RFC: replace pg_stat_activity.waiting with something more descriptive |
Previous Message | Michael Paquier | 2015-12-24 02:26:27 | Re: proposal: function parse_ident |