No, pg_size_pretty(numeric) was not such a hot idea

In 9.1:

regression=# select pg_size_pretty(8*1024*1024);
pg_size_pretty
----------------
8192 kB
(1 row)

In HEAD:

regression=# select pg_size_pretty(8*1024*1024);
ERROR: function pg_size_pretty(integer) is not unique
LINE 1: select pg_size_pretty(8*1024*1024);
^
HINT: Could not choose a best candidate function. You might need to add explicit type casts.

The argument for adding pg_size_pretty(numeric) was pretty darn thin in
the first place, IMHO; it does not seem to me that it justified this
loss of usability.

regards, tom lane

On Sat, May 26, 2012 at 9:30 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> In 9.1:
>
> regression=# select pg_size_pretty(8*1024*1024);
>  pg_size_pretty
> ----------------
>  8192 kB
> (1 row)
>
> In HEAD:
>
> regression=# select pg_size_pretty(8*1024*1024);
> ERROR:  function pg_size_pretty(integer) is not unique
> LINE 1: select pg_size_pretty(8*1024*1024);
>               ^
> HINT:  Could not choose a best candidate function. You might need to add explicit type casts.
>
> The argument for adding pg_size_pretty(numeric) was pretty darn thin in
> the first place, IMHO; it does not seem to me that it justified this
> loss of usability.

Ouch! But removing pg_size_pretty(numeric) causes another usability
issue, e.g., pg_size_pretty(pg_xlog_location_diff(...)) fails. So how about
removing pg_size_pretty(bigint) to resolve those two issues?
I guess pg_size_pretty(numeric) is a bit slower than bigint version, but
I don't think that such a bit slowdown of pg_size_pretty() becomes
a matter practically. No?

Regards,

--
Fujii Masao

On 26-05-2012 01:45, Fujii Masao wrote:
> Ouch! But removing pg_size_pretty(numeric) causes another usability
> issue, e.g., pg_size_pretty(pg_xlog_location_diff(...)) fails. So how about
> removing pg_size_pretty(bigint) to resolve those two issues?
> I guess pg_size_pretty(numeric) is a bit slower than bigint version, but
> I don't think that such a bit slowdown of pg_size_pretty() becomes
> a matter practically. No?
>
That's what I proposed at [1]. +1 for dropping the pg_size_pretty(bigint).

[1] http://archives.postgresql.org/message-id/4F315F6C.8030700@timbira.com

--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> On Sat, May 26, 2012 at 9:30 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> The argument for adding pg_size_pretty(numeric) was pretty darn thin in
>> the first place, IMHO; it does not seem to me that it justified this
>> loss of usability.

> Ouch! But removing pg_size_pretty(numeric) causes another usability
> issue, e.g., pg_size_pretty(pg_xlog_location_diff(...)) fails. So how about
> removing pg_size_pretty(bigint) to resolve those two issues?
> I guess pg_size_pretty(numeric) is a bit slower than bigint version, but
> I don't think that such a bit slowdown of pg_size_pretty() becomes
> a matter practically. No?

AFAICS that argument is based on wishful thinking, not evidence.

I did some simple measurements and determined that at least on my
development machine, pg_size_pretty(numeric) is about a factor of four
slower than pg_size_pretty(bigint) --- and that's just counting the
function itself, not any added coercion-to-numeric processing. Now
maybe you could argue that it's never going to be used in a context
where anyone cares about its performance at all, but I've got doubts
about that.

In any case, it's probably too late to do anything about this for 9.2;
and once we ship it like that there will be little point in changing
it later, since people will already have had to add explicit casts
to any queries where the problem arises.

regards, tom lane

On Sun, May 27, 2012 at 1:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>> On Sat, May 26, 2012 at 9:30 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> The argument for adding pg_size_pretty(numeric) was pretty darn thin in
>>> the first place, IMHO; it does not seem to me that it justified this
>>> loss of usability.
>
>> Ouch! But removing pg_size_pretty(numeric) causes another usability
>> issue, e.g., pg_size_pretty(pg_xlog_location_diff(...)) fails. So how about
>> removing pg_size_pretty(bigint) to resolve those two issues?
>> I guess pg_size_pretty(numeric) is a bit slower than bigint version, but
>> I don't think that such a bit slowdown of pg_size_pretty() becomes
>> a matter practically. No?
>
> AFAICS that argument is based on wishful thinking, not evidence.
>
> I did some simple measurements and determined that at least on my
> development machine, pg_size_pretty(numeric) is about a factor of four
> slower than pg_size_pretty(bigint) --- and that's just counting the
> function itself, not any added coercion-to-numeric processing.  Now
> maybe you could argue that it's never going to be used in a context
> where anyone cares about its performance at all, but I've got doubts
> about that.

OK, let me propose another approach: add pg_size_pretty(int).
If we do this, all usability and performance problems will be solved.
Thought?

Attached patch adds pg_size_pretty(int).

Regards,

--
Fujii Masao

On 27-05-2012 10:45, Fujii Masao wrote:
> OK, let me propose another approach: add pg_size_pretty(int).
> If we do this, all usability and performance problems will be solved.
>
I wouldn't like to add another function but if it solves both problems... +1.

--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On 5/27/12 2:54 PM, Euler Taveira wrote:
> On 27-05-2012 10:45, Fujii Masao wrote:
>> OK, let me propose another approach: add pg_size_pretty(int).
>> If we do this, all usability and performance problems will be solved.
>>
> I wouldn't like to add another function but if it solves both problems... +1.

FWIW, I would argue that the case of pg_size_pretty(8*1024*1024) is pretty contrived... when would you actually do something like that? ISTM that any time you're using pg_size_pretty you'd be coming off a real datatype.
--
Jim C. Nasby, Database Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

Jim Nasby <jim(at)nasby(dot)net> writes:
> On 5/27/12 2:54 PM, Euler Taveira wrote:
>> On 27-05-2012 10:45, Fujii Masao wrote:
>>> OK, let me propose another approach: add pg_size_pretty(int).

>> I wouldn't like to add another function but if it solves both problems... +1.

> FWIW, I would argue that the case of pg_size_pretty(8*1024*1024) is
> pretty contrived...

Yeah, possibly. In any case, I don't think we're making either of these
changes in 9.2, because the time for forcing initdbs is past. It would
only be realistic to think about changing pg_size_pretty() if we come
across some other, much more compelling reason to force a system catalog
contents change.

Assuming that's how 9.2 ships, we might as well wait to see if there
are any real complaints from the field before we decide whether any
changing is needed.

regards, tom lane

On Tue, Jun 5, 2012 at 1:01 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Jim Nasby <jim(at)nasby(dot)net> writes:
>> On 5/27/12 2:54 PM, Euler Taveira wrote:
>>> On 27-05-2012 10:45, Fujii Masao wrote:
>>>> OK, let me propose another approach: add pg_size_pretty(int).
>
>>> I wouldn't like to add another function but if it solves both problems... +1.
>
>> FWIW, I would argue that the case of pg_size_pretty(8*1024*1024) is
>> pretty contrived...
>
> Yeah, possibly.  In any case, I don't think we're making either of these
> changes in 9.2, because the time for forcing initdbs is past.  It would
> only be realistic to think about changing pg_size_pretty() if we come
> across some other, much more compelling reason to force a system catalog
> contents change.
>
> Assuming that's how 9.2 ships, we might as well wait to see if there
> are any real complaints from the field before we decide whether any
> changing is needed.

We could add it to the catalog without forcing an initdb. That way
anybody who installed on the release (or beta3+) would get the
function, and not those who started on the beta (unless they created
it manually). That does leave us in a position where people have
different versions of the schema with the same identifier though, so
that may not be the best idea.

If we're just leaving it, should we take it off the open items list,
or leave it in there "in case something else shows up"?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Tue, Jun 5, 2012 at 1:01 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Assuming that's how 9.2 ships, we might as well wait to see if there
>> are any real complaints from the field before we decide whether any
>> changing is needed.

> We could add it to the catalog without forcing an initdb.

Ugh.

> If we're just leaving it, should we take it off the open items list,
> or leave it in there "in case something else shows up"?

Let's just take it off the list.

regards, tom lane

On Tue, Jun 5, 2012 at 8:01 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Jim Nasby <jim(at)nasby(dot)net> writes:
>> On 5/27/12 2:54 PM, Euler Taveira wrote:
>>> On 27-05-2012 10:45, Fujii Masao wrote:
>>>> OK, let me propose another approach: add pg_size_pretty(int).
>
>>> I wouldn't like to add another function but if it solves both problems... +1.
>
>> FWIW, I would argue that the case of pg_size_pretty(8*1024*1024) is
>> pretty contrived...
>
> Yeah, possibly.  In any case, I don't think we're making either of these
> changes in 9.2, because the time for forcing initdbs is past.  It would
> only be realistic to think about changing pg_size_pretty() if we come
> across some other, much more compelling reason to force a system catalog
> contents change.
>
> Assuming that's how 9.2 ships, we might as well wait to see if there
> are any real complaints from the field before we decide whether any
> changing is needed.

We cannot change a system catalog content at all. So, I'm worried
that we receive such complaints after the release of 9.2 but cannot
address that until 9.3.

Regards,

--
Fujii Masao

>> Assuming that's how 9.2 ships, we might as well wait to see if there
>> are any real complaints from the field before we decide whether any
>> changing is needed.

So, here's a complaint: 9.2 is breaking our code for checking table sizes:

postgres=# select pg_size_pretty(100);
ERROR: function pg_size_pretty(integer) is not unique at character 8
HINT: Could not choose a best candidate function. You might need to add
explicit type casts.
STATEMENT: select pg_size_pretty(100);
ERROR: function pg_size_pretty(integer) is not unique
LINE 1: select pg_size_pretty(100);
^
HINT: Could not choose a best candidate function. You might need to add
explicit type casts.

Obviously, we can work around it though. Let's see if anyone else
complains ...

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Oct 10, 2012 at 2:49 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> So, here's a complaint: 9.2 is breaking our code for checking table sizes:
>
> postgres=# select pg_size_pretty(100);
> ERROR: function pg_size_pretty(integer) is not unique at character 8

You know, if we implemented what Tom proposed here:

http://archives.postgresql.org/pgsql-hackers/2012-08/msg01055.php

...then we probably get away with removing pg_size_pretty(bigint) and
then this would Just Work. pg_size_pretty(numeric) is doubtless a
little slower than pg_size_pretty(bigint), but I think in practice
nobody's going to care.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Oct 10, 2012 at 11:49:50AM -0700, Josh Berkus wrote:
>
> >> Assuming that's how 9.2 ships, we might as well wait to see if there
> >> are any real complaints from the field before we decide whether any
> >> changing is needed.
>
> So, here's a complaint: 9.2 is breaking our code for checking table sizes:
>
> postgres=# select pg_size_pretty(100);
> ERROR: function pg_size_pretty(integer) is not unique at character 8
> HINT: Could not choose a best candidate function. You might need to add
> explicit type casts.
> STATEMENT: select pg_size_pretty(100);
> ERROR: function pg_size_pretty(integer) is not unique
> LINE 1: select pg_size_pretty(100);
> ^
> HINT: Could not choose a best candidate function. You might need to add
> explicit type casts.
>
> Obviously, we can work around it though. Let's see if anyone else
> complains ...

Where are we on this? I still see this behavior:

test=> SELECT pg_size_pretty(100);
ERROR: function pg_size_pretty(integer) is not unique
LINE 1: SELECT pg_size_pretty(100);
^
HINT: Could not choose a best candidate function. You might need to add explicit type casts.

\df shows:

test=> \df pg_size_pretty
List of functions
Schema | Name | Result data type | Argument data types | Type
------------+----------------+------------------+---------------------+--------
pg_catalog | pg_size_pretty | text | bigint | normal
pg_catalog | pg_size_pretty | text | numeric | normal
(2 rows)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Was this issue ever resolved?
We are now having Nagios checks failing due to the pg_size_pretty function,
and the check runs fine on my local machine 9.1 (fails on 9.2 and 9.3, both
having two pg_size_pretty functions).

Thanks,
Josh

--
View this message in context: http://postgresql.1045698.n5.nabble.com/No-pg-size-pretty-numeric-was-not-such-a-hot-idea-tp5710106p5813345.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Josh Loberant <jamracing(at)gmail(dot)com> writes:
> Was this issue ever resolved?
> We are now having Nagios checks failing due to the pg_size_pretty function,
> and the check runs fine on my local machine 9.1 (fails on 9.2 and 9.3, both
> having two pg_size_pretty functions).

Nothing was done about it so far for lack of consensus.

Given that there are now three release branches that behave like this,
fixing the Nagios check seems like the advisable answer. Just cast the
argument to bigint (or numeric, if that seems like a better idea).

regards, tom lane