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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to