Re: [BUGS] BUG #12989: pg_size_pretty with negative values

From: "Adrian(dot)Vondendriesch" <Adrian(dot)Vondendriesch(at)credativ(dot)de>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>
Cc: "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Christian Almeida <cbalmeida(at)gmail(dot)com>
Subject: Re: [BUGS] BUG #12989: pg_size_pretty with negative values
Date: 2015-11-05 21:19:34
Message-ID: 563BC7E6.9060101@credativ.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

New patch attached and rebased on HEAD
(8c75ad436f75fc629b61f601ba884c8f9313c9af).

Am 03.11.2015 um 04:06 schrieb Robert Haas:
> On Sat, Oct 31, 2015 at 2:25 PM, Julien Rouhaud
> <julien(dot)rouhaud(at)dalibo(dot)com> wrote:
>> I just reviewed your patch, everything looks fine for me. Maybe some
>> minor cosmetic changes could be made to avoid declaring too many vars,
>> but I think a committer would have a better idea on this, so I mark
>> this patch as ready for committer.
>
> I don't think we should define Sign(x) as a macro in c.h. c.h is
> really only supposed to contain stuff that's pretty generic and
> universal, and the fact that we haven't needed it up until now
> suggests that Sign(x) isn't. I'd suggest instead defining something
> like:
>
> #define half_rounded(x) (((x) + (x) < 0 ? 0 : 1) / 2)

I removed the Sign macro and introduced half_rounded. It's placed it
in dbsize.c. Please let me know if that's the wrong place.

>
> Maybe rename numeric_divide_by_two to numeric_half_rounded.
> Generally, let's try to make the numeric and int64 code paths look as
> similar as possible.

I renamed numeric_divide_by_two.

>
> Recomputing numeric_absolute(x) multiple times should be avoided.
> Compute it once and save the answer.

Because "size" is shifted multiple times within pg_size_pretty and
pg_size_pretty_numeric the absolute values needs to be recomputed.
Please let me know if I oversee something.

I changed the status back to "needs review".

Regards,
- Adrian

Attachment Content-Type Size
0001-Make-pg_size_pretty-handle-negative-values-v2.patch text/x-diff 12.6 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2015-11-05 23:18:45 Re: BUG #13756: jsonb_path_ops gin index produce empty result on nested array
Previous Message Peter Geoghegan 2015-11-05 20:32:40 Re: BUG #13756: jsonb_path_ops gin index produce empty result on nested array

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-11-05 21:19:36 Re: extend pgbench expressions with functions
Previous Message Robert Haas 2015-11-05 21:15:44 Re: [PATCH] Skip ALTER x SET SCHEMA if the schema didn't change