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

On 20/09/2015 14:16, Adrian.Vondendriesch wrote:
> Hi all,
>

Hello,

> Am 06.04.2015 um 20:52 schrieb Tom Lane:
>> "David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
>>> I'll let a hacker determine whether this is a bug or a feature
>>> request though it is a POLA violation in either case.
>>
>> I'd say it's a feature request --- a perfectly reasonable one,
>> but I doubt we'd alter the behavior of the function in the back
>> branches.
>
> I was also wondering about the described behaviour. IMO
> pg_size_pretty should handle negative values the same way as
> positive values are handled.
>

+1 for me, thanks for the patch!

> I've attached a patch which implements the requested behaviour.
> The patch applies clean to HEAD (currently 85eda7e92).
>

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.

> AFAICS the documentation doesn't say anything about pg_size_pretty
> and negative values. So, I didn't touch the documentation. If this
> is a oversight by me or should be documented after all, I will
> provide a additional documentation patch.
>

Yes, the documentation didn't say anything about the lack of
formattting for negative value. I also don't think a patch is needed here.

Regards.

> Before the patch:
>
>> SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
>> (SELECT 100000000000000::bigint) foo(size); pg_size_pretty |
>> pg_size_pretty ----------------+------------------------ 91 TB
>> | -100000000000000 bytes (1 row)
>
>> SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
>> (SELECT 100000000000000::numeric) foo(size); pg_size_pretty |
>> pg_size_pretty ----------------+------------------------ 91 TB
>> | -100000000000000 bytes (1 row)
>
> After the patch:
>
>> SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
>> (SELECT 100000000000000::bigint) foo(size); pg_size_pretty |
>> pg_size_pretty ----------------+---------------- 91 TB |
>> -91 TB (1 row)
>
>> SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
>> (SELECT 100000000000000::numeric) foo(size); pg_size_pretty |
>> pg_size_pretty ----------------+---------------- 91 TB |
>> -91 TB (1 row)
>
>
> The patch contains two tests (pg_size_pretty_bigint and
> pg_size_pretty_numeric), to verify that positive and negative
> values return the same result (except sign).
>
> Greetings,
>
> - Adrian
>

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org