Re: [BUGS] BUG #12989: pg_size_pretty with negative values - Mailing list pgsql-hackers

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.g.johnston@gmail.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