New patch attached and rebased on HEAD (8c75ad436f75fc629b61f601ba884c8f9313c9af).
Am 03.11.2015 um 04:06 schrieb Robert Haas: > On Sat, Oct 31, 2015 at 2:25 PM, Julien Rouhaud > <julien.rouh...@dalibo.com> wrote: >> 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. > > I don't think we should define Sign(x) as a macro in c.h. c.h is > really only supposed to contain stuff that's pretty generic and > universal, and the fact that we haven't needed it up until now > suggests that Sign(x) isn't. I'd suggest instead defining something > like: > > #define half_rounded(x) (((x) + (x) < 0 ? 0 : 1) / 2) I removed the Sign macro and introduced half_rounded. It's placed it in dbsize.c. Please let me know if that's the wrong place. > > Maybe rename numeric_divide_by_two to numeric_half_rounded. > Generally, let's try to make the numeric and int64 code paths look as > similar as possible. I renamed numeric_divide_by_two. > > Recomputing numeric_absolute(x) multiple times should be avoided. > Compute it once and save the answer. Because "size" is shifted multiple times within pg_size_pretty and pg_size_pretty_numeric the absolute values needs to be recomputed. Please let me know if I oversee something. I changed the status back to "needs review". Regards, - Adrian
From c06d1463cf21957260327c47217050e334058422 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 | 67 ++++++++++++-------
.../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 +++++++++
5 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..84c67d3 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -31,6 +31,12 @@
#include "utils/relmapper.h"
#include "utils/syscache.h"
+/*
+ * half_rounded
+ * Return (x / 2) if x is smaller than 0
+ * ((x + 1) / 2) otherwise.
+ */
+#define half_rounded(x) (((x) + ((x) < 0 ? 0 : 1)) / 2)
/* Return physical size of directory contents, or 0 if dir doesn't exist */
static int64
@@ -534,31 +540,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);
+ half_rounded(size));
else
{
size >>= 10;
- if (size < limit2)
+ if (Abs(size) < limit2)
snprintf(buf, sizeof(buf), INT64_FORMAT " MB",
- (size + 1) / 2);
+ half_rounded(size));
else
{
size >>= 10;
- if (size < limit2)
+ if (Abs(size) < limit2)
snprintf(buf, sizeof(buf), INT64_FORMAT " GB",
- (size + 1) / 2);
+ half_rounded(size));
else
{
size >>= 10;
snprintf(buf, sizeof(buf), INT64_FORMAT " TB",
- (size + 1) / 2);
+ half_rounded(size));
}
}
}
@@ -593,16 +599,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_half_rounded(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 +659,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 +669,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_half_rounded(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_half_rounded(size);
result = psprintf("%s MB", numeric_to_cstring(size));
}
else
@@ -663,18 +688,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_half_rounded(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_half_rounded(size);
result = psprintf("%s TB", numeric_to_cstring(size));
}
}
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.6.2
signature.asc
Description: OpenPGP digital signature
