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@sss.pgh.pa.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]http://archives.postgresql.org/message-id/4F315F6C.8030700@timbira.com. +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.fujii@gmail.com> writes:
On Sat, May 26, 2012 at 9:30 AM, Tom Lane <tgl@sss.pgh.pa.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@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Sat, May 26, 2012 at 9:30 AM, Tom Lane <tgl@sss.pgh.pa.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
Attachments:
size_pretty_int4_v1.patchapplication/octet-stream; name=size_pretty_int4_v1.patchDownload
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***************
*** 14978,14983 **** postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
--- 14978,14993 ----
</row>
<row>
<entry>
+ <literal><function>pg_size_pretty(<type>int</type>)</function></literal>
+ </entry>
+ <entry><type>text</type></entry>
+ <entry>
+ Converts a size in bytes expressed as a 32-bit integer into a
+ human-readable format with size units
+ </entry>
+ </row>
+ <row>
+ <entry>
<literal><function>pg_size_pretty(<type>bigint</type>)</function></literal>
</entry>
<entry><type>text</type></entry>
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
***************
*** 551,556 **** pg_size_pretty(PG_FUNCTION_ARGS)
--- 551,597 ----
PG_RETURN_TEXT_P(cstring_to_text(buf));
}
+ Datum
+ pg_size_pretty_int4(PG_FUNCTION_ARGS)
+ {
+ int32 size = PG_GETARG_INT32(0);
+ char buf[64];
+ int32 limit = 10 * 1024;
+ int32 limit2 = limit * 2 - 1;
+
+ if (size < limit)
+ snprintf(buf, sizeof(buf), "%d bytes", size);
+ else
+ {
+ size >>= 9; /* keep one extra bit for rounding */
+ if (size < limit2)
+ snprintf(buf, sizeof(buf), "%d kB",
+ (size + 1) / 2);
+ else
+ {
+ size >>= 10;
+ if (size < limit2)
+ snprintf(buf, sizeof(buf), "%d MB",
+ (size + 1) / 2);
+ else
+ {
+ size >>= 10;
+ if (size < limit2)
+ snprintf(buf, sizeof(buf), "%d GB",
+ (size + 1) / 2);
+ else
+ {
+ size >>= 10;
+ snprintf(buf, sizeof(buf), "%d TB",
+ (size + 1) / 2);
+ }
+ }
+ }
+ }
+
+ PG_RETURN_TEXT_P(cstring_to_text(buf));
+ }
+
static char *
numeric_to_cstring(Numeric n)
{
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 3410,3415 **** DATA(insert OID = 2286 ( pg_total_relation_size PGNSP PGUID 12 1 0 0 0 f f f f t
--- 3410,3417 ----
DESCR("total disk space usage for the specified table and associated indexes");
DATA(insert OID = 2288 ( pg_size_pretty PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "20" _null_ _null_ _null_ _null_ pg_size_pretty _null_ _null_ _null_ ));
DESCR("convert a long int to a human readable text using size units");
+ DATA(insert OID = 3167 ( pg_size_pretty PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "23" _null_ _null_ _null_ _null_ pg_size_pretty_int4 _null_ _null_ _null_ ));
+ DESCR("convert an int to a human readable text using size units");
DATA(insert OID = 3166 ( pg_size_pretty PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "1700" _null_ _null_ _null_ _null_ pg_size_pretty_numeric _null_ _null_ _null_ ));
DESCR("convert a numeric to a human readable text using size units");
DATA(insert OID = 2997 ( pg_table_size PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 20 "2205" _null_ _null_ _null_ _null_ pg_table_size _null_ _null_ _null_ ));
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 453,458 **** extern Datum pg_database_size_name(PG_FUNCTION_ARGS);
--- 453,459 ----
extern Datum pg_relation_size(PG_FUNCTION_ARGS);
extern Datum pg_total_relation_size(PG_FUNCTION_ARGS);
extern Datum pg_size_pretty(PG_FUNCTION_ARGS);
+ extern Datum pg_size_pretty_int4(PG_FUNCTION_ARGS);
extern Datum pg_size_pretty_numeric(PG_FUNCTION_ARGS);
extern Datum pg_table_size(PG_FUNCTION_ARGS);
extern Datum pg_indexes_size(PG_FUNCTION_ARGS);
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@nasby.net
512.569.9461 (cell) http://jim.nasby.net
Jim Nasby <jim@nasby.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@sss.pgh.pa.us> wrote:
Jim Nasby <jim@nasby.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@hagander.net> writes:
On Tue, Jun 5, 2012 at 1:01 AM, Tom Lane <tgl@sss.pgh.pa.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@sss.pgh.pa.us> wrote:
Jim Nasby <jim@nasby.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@agliodbs.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Josh Loberant <jamracing@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers