From: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: partition table and stddev() /variance() behaviour |
Date: | 2018-06-21 15:11:20 |
Message-ID: | CAKJS1f-gQNZscEbX7t0u7V-D_jEh9qEXhnwtBXev5pdgZM+L3Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 22 June 2018 at 02:01, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
>> Well, that's quite surprising. It appears to be a bug in
>> numeric_poly_combine for machines without a working int128 type. The
>> parameters in accum_sum_copy are in the incorrect order.
>
> Ouch.
Yeah. Looks like this function was correct when it went in. It was
9cca11c915e (v10) that caused the issue.
>> The very minimal fix is attached, but I'll need to go look at where
>> the tests for this have gone.
>
> coverage.postgresql.org shows that numeric_poly_serialize/combine()
> aren't exercised at all by the regression tests. Which is embarrassing
> for this case, but I'm a bit leery of trying to insist on 100% coverage.
>
> It might be a plan to insist on buildfarm coverage for anything with
> platform-varying code in it, in which case there's at least one
> other undertested bit of HAVE_INT128 code in numeric.c.
I'm not familiar with what the coverage tool can do, so maybe there's
an easier way than running a script to check for 0 coverage between
#ifdef / #if and #endif inside all .c files.
This sounds like a longer-term project though, let's get this one fixed first.
I think some coverage of the numerical aggregates is a good idea, so
I've added some in the attached. I managed to get a parallel plan
going with a query to onek, which is pretty cheap to execute. I didn't
touch the bool aggregates. Maybe I should have done that too..?
I also did a quick validation of the other accum_sum_copy usages. All
others look correct.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
fix_numeric_poly_combine_v2.patch | application/octet-stream | 18.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2018-06-21 15:16:51 | Re: partition table and stddev() /variance() behaviour |
Previous Message | Bruce Momjian | 2018-06-21 15:09:27 | Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS) |