Hi all, Am 06.04.2015 um 20:52 schrieb Tom Lane: > "David G. Johnston" <david.g.johns...@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. I've attached a patch which implements the requested behaviour. The patch applies clean to HEAD (currently 85eda7e92). 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. 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
From 08ae6cdeaf261e156ce5f7622f0d7426c1249485 Mon Sep 17 00:00:00 2001 From: Adrian Vondendriesch <adrian.vondendrie...@credativ.de> Date: Sat, 19 Sep 2015 15:54:13 +0200 Subject: [PATCH] Make pg_size_pretty handle negative values. Make pg_size_pretty(bigint) and pg_size_pretty(numeric) handle negative values the same way as positive values are. This commit introduces a new macro "Sign", which is used within pg_size_pretty(bigint). Also numeric_plus_one_over_two is renamed to numeric_divide_by_two. numeric_divide_by_two now handles negative values the same way as positive values are handled. To get the absolute value of a Numeric variable, a new static function called numeric_absolute is introduced. --- src/backend/utils/adt/dbsize.c | 61 +++++++++++------- src/include/c.h | 6 ++ .../regress/expected/pg_size_pretty_bigint.out | 39 +++++++++++ .../regress/expected/pg_size_pretty_numeric.out | 75 ++++++++++++++++++++++ src/test/regress/sql/pg_size_pretty_bigint.sql | 15 +++++ src/test/regress/sql/pg_size_pretty_numeric.sql | 29 +++++++++ 6 files changed, 203 insertions(+), 22 deletions(-) create mode 100644 src/test/regress/expected/pg_size_pretty_bigint.out create mode 100644 src/test/regress/expected/pg_size_pretty_numeric.out create mode 100644 src/test/regress/sql/pg_size_pretty_bigint.sql create mode 100644 src/test/regress/sql/pg_size_pretty_numeric.sql diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 82311b4..97095bb 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -534,31 +534,31 @@ pg_size_pretty(PG_FUNCTION_ARGS) int64 limit = 10 * 1024; int64 limit2 = limit * 2 - 1; - if (size < limit) + if (Abs(size) < limit) snprintf(buf, sizeof(buf), INT64_FORMAT " bytes", size); else { size >>= 9; /* keep one extra bit for rounding */ - if (size < limit2) + if (Abs(size) < limit2) snprintf(buf, sizeof(buf), INT64_FORMAT " kB", - (size + 1) / 2); + (size + Sign(size)) / 2); else { size >>= 10; - if (size < limit2) + if (Abs(size) < limit2) snprintf(buf, sizeof(buf), INT64_FORMAT " MB", - (size + 1) / 2); + (size + Sign(size)) / 2); else { size >>= 10; - if (size < limit2) + if (Abs(size) < limit2) snprintf(buf, sizeof(buf), INT64_FORMAT " GB", - (size + 1) / 2); + (size + Sign(size)) / 2); else { size >>= 10; snprintf(buf, sizeof(buf), INT64_FORMAT " TB", - (size + 1) / 2); + (size + Sign(size)) / 2); } } } @@ -593,16 +593,37 @@ numeric_is_less(Numeric a, Numeric b) } static Numeric -numeric_plus_one_over_two(Numeric n) +numeric_absolute(Numeric n) { Datum d = NumericGetDatum(n); + Datum result; + + result = DirectFunctionCall1(numeric_abs, d); + return DatumGetNumeric(result); +} + +static Numeric +numeric_divide_by_two(Numeric n) +{ + Datum d = NumericGetDatum(n); + Datum zero; Datum one; Datum two; Datum result; + bool greater_or_equal_zero; + zero = DirectFunctionCall1(int8_numeric, Int64GetDatum(0)); one = DirectFunctionCall1(int8_numeric, Int64GetDatum(1)); two = DirectFunctionCall1(int8_numeric, Int64GetDatum(2)); - result = DirectFunctionCall2(numeric_add, d, one); + + result = DirectFunctionCall2(numeric_ge, d, zero); + greater_or_equal_zero = DatumGetBool(result); + + if (greater_or_equal_zero) + result = DirectFunctionCall2(numeric_add, d, one); + else + result = DirectFunctionCall2(numeric_sub, d, one); + result = DirectFunctionCall2(numeric_div_trunc, result, two); return DatumGetNumeric(result); } @@ -632,7 +653,7 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS) limit = int64_to_numeric(10 * 1024); limit2 = int64_to_numeric(10 * 1024 * 2 - 1); - if (numeric_is_less(size, limit)) + if (numeric_is_less(numeric_absolute(size), limit)) { result = psprintf("%s bytes", numeric_to_cstring(size)); } @@ -642,20 +663,18 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS) /* size >>= 9 */ size = numeric_shift_right(size, 9); - if (numeric_is_less(size, limit2)) + if (numeric_is_less(numeric_absolute(size), limit2)) { - /* size = (size + 1) / 2 */ - size = numeric_plus_one_over_two(size); + size = numeric_divide_by_two(size); result = psprintf("%s kB", numeric_to_cstring(size)); } else { /* size >>= 10 */ size = numeric_shift_right(size, 10); - if (numeric_is_less(size, limit2)) + if (numeric_is_less(numeric_absolute(size), limit2)) { - /* size = (size + 1) / 2 */ - size = numeric_plus_one_over_two(size); + size = numeric_divide_by_two(size); result = psprintf("%s MB", numeric_to_cstring(size)); } else @@ -663,18 +682,16 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS) /* size >>= 10 */ size = numeric_shift_right(size, 10); - if (numeric_is_less(size, limit2)) + if (numeric_is_less(numeric_absolute(size), limit2)) { - /* size = (size + 1) / 2 */ - size = numeric_plus_one_over_two(size); + size = numeric_divide_by_two(size); result = psprintf("%s GB", numeric_to_cstring(size)); } else { /* size >>= 10 */ size = numeric_shift_right(size, 10); - /* size = (size + 1) / 2 */ - size = numeric_plus_one_over_two(size); + size = numeric_divide_by_two(size); result = psprintf("%s TB", numeric_to_cstring(size)); } } diff --git a/src/include/c.h b/src/include/c.h index 8163b00..1fb16f1 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -793,6 +793,12 @@ typedef NameData *Name; #define Abs(x) ((x) >= 0 ? (x) : -(x)) /* + * Sign + * Return one if a number is positive 0 otherwise. + */ +#define Sign(x) ((x) >= 0 ? 1 : 0) + +/* * StrNCpy * Like standard library function strncpy(), except that result string * is guaranteed to be null-terminated --- that is, at most N-1 bytes diff --git a/src/test/regress/expected/pg_size_pretty_bigint.out b/src/test/regress/expected/pg_size_pretty_bigint.out new file mode 100644 index 0000000..f669e35 --- /dev/null +++ b/src/test/regress/expected/pg_size_pretty_bigint.out @@ -0,0 +1,39 @@ +-- +-- pg_size_pretty_bigint +-- +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 10::bigint) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 10 bytes | -10 bytes +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000::bigint) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 1000 bytes | -1000 bytes +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000::bigint) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 977 kB | -977 kB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000::bigint) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 954 MB | -954 MB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000::bigint) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 931 GB | -931 GB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000000::bigint) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 909 TB | -909 TB +(1 row) + diff --git a/src/test/regress/expected/pg_size_pretty_numeric.out b/src/test/regress/expected/pg_size_pretty_numeric.out new file mode 100644 index 0000000..ce34952 --- /dev/null +++ b/src/test/regress/expected/pg_size_pretty_numeric.out @@ -0,0 +1,75 @@ +-- +-- pg_size_pretty_numeric +-- +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 10::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 10 bytes | -10 bytes +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 1000 bytes | -1000 bytes +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 977 kB | -977 kB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 954 MB | -954 MB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 931 GB | -931 GB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000000::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 909 TB | -909 TB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 10.5::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 10.5 bytes | -10.5 bytes +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000.5::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 1000.5 bytes | -1000.5 bytes +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000.5::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 977 kB | -977 kB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000.5::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 954 MB | -954 MB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000.5::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 931 GB | -931 GB +(1 row) + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000000.5::numeric) pg_size_pretty(size); + pg_size_pretty | pg_size_pretty +----------------+---------------- + 909 TB | -909 TB +(1 row) + diff --git a/src/test/regress/sql/pg_size_pretty_bigint.sql b/src/test/regress/sql/pg_size_pretty_bigint.sql new file mode 100644 index 0000000..a232203 --- /dev/null +++ b/src/test/regress/sql/pg_size_pretty_bigint.sql @@ -0,0 +1,15 @@ +-- +-- pg_size_pretty_bigint +-- + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 10::bigint) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000::bigint) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000::bigint) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000::bigint) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000::bigint) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000000::bigint) pg_size_pretty(size); diff --git a/src/test/regress/sql/pg_size_pretty_numeric.sql b/src/test/regress/sql/pg_size_pretty_numeric.sql new file mode 100644 index 0000000..c1b626e --- /dev/null +++ b/src/test/regress/sql/pg_size_pretty_numeric.sql @@ -0,0 +1,29 @@ + +-- +-- pg_size_pretty_numeric +-- + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 10::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000000::numeric) pg_size_pretty(size); + + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 10.5::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000.5::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000.5::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000.5::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000.5::numeric) pg_size_pretty(size); + +SELECT pg_size_pretty(size), pg_size_pretty(-1 * size) FROM (SELECT 1000000000000000.5::numeric) pg_size_pretty(size); -- 2.5.1
signature.asc
Description: OpenPGP digital signature