Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 674ee3b

Browse filesBrowse files
david-rowleydavid-rowley
committed
Fix incorrect return value in pg_size_pretty(bigint)
Due to how pg_size_pretty(bigint) was implemented, it's possible that when given a negative number of bytes that the returning value would not match the equivalent positive return value when given the equivalent positive number of bytes. This was due to two separate issues. 1. The function used bit shifting to convert the number of bytes into larger units. The rounding performed by bit shifting is not the same as dividing. For example -3 >> 1 = -2, but -3 / 2 = -1. These two operations are only equivalent with positive numbers. 2. The half_rounded() macro rounded towards positive infinity. This meant that negative numbers rounded towards zero and positive numbers rounded away from zero. Here we fix #1 by dividing the values instead of bit shifting. We fix #2 by adjusting the half_rounded macro always to round away from zero. Additionally, adjust the pg_size_pretty(numeric) function to be more explicit that it's using division rather than bit shifting. A casual observer might have believed bit shifting was used due to a static function being named numeric_shift_right. However, that function was calculating the divisor from the number of bits and performed division. Here we make that more clear. This change is just cosmetic and does not affect the return value of the numeric version of the function. Here we also add a set of regression tests both versions of pg_size_pretty() which test the values directly before and after the function switches to the next unit. This bug was introduced in 8a1fab3. Prior to that negative values were always displayed in bytes. Author: Dean Rasheed, David Rowley Discussion: https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.com Backpatch-through: 9.6, where the bug was introduced.
1 parent 67f925b commit 674ee3b
Copy full SHA for 674ee3b

File tree

3 files changed

+79
-18
lines changed
Filter options

3 files changed

+79
-18
lines changed

‎src/backend/utils/adt/dbsize.c

Copy file name to clipboardExpand all lines: src/backend/utils/adt/dbsize.c
+21-18Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
#include "utils/relmapper.h"
3232
#include "utils/syscache.h"
3333

34-
/* Divide by two and round towards positive infinity. */
35-
#define half_rounded(x) (((x) + ((x) < 0 ? 0 : 1)) / 2)
34+
/* Divide by two and round away from zero */
35+
#define half_rounded(x) (((x) + ((x) < 0 ? -1 : 1)) / 2)
3636

3737
/* Return physical size of directory contents, or 0 if dir doesn't exist */
3838
static int64
@@ -540,25 +540,29 @@ pg_size_pretty(PG_FUNCTION_ARGS)
540540
snprintf(buf, sizeof(buf), INT64_FORMAT " bytes", size);
541541
else
542542
{
543-
size >>= 9; /* keep one extra bit for rounding */
543+
/*
544+
* We use divide instead of bit shifting so that behavior matches for
545+
* both positive and negative size values.
546+
*/
547+
size /= (1 << 9); /* keep one extra bit for rounding */
544548
if (Abs(size) < limit2)
545549
snprintf(buf, sizeof(buf), INT64_FORMAT " kB",
546550
half_rounded(size));
547551
else
548552
{
549-
size >>= 10;
553+
size /= (1 << 10);
550554
if (Abs(size) < limit2)
551555
snprintf(buf, sizeof(buf), INT64_FORMAT " MB",
552556
half_rounded(size));
553557
else
554558
{
555-
size >>= 10;
559+
size /= (1 << 10);
556560
if (Abs(size) < limit2)
557561
snprintf(buf, sizeof(buf), INT64_FORMAT " GB",
558562
half_rounded(size));
559563
else
560564
{
561-
size >>= 10;
565+
size /= (1 << 10);
562566
snprintf(buf, sizeof(buf), INT64_FORMAT " TB",
563567
half_rounded(size));
564568
}
@@ -627,15 +631,13 @@ numeric_half_rounded(Numeric n)
627631
}
628632

629633
static Numeric
630-
numeric_shift_right(Numeric n, unsigned count)
634+
numeric_truncated_divide(Numeric n, int64 divisor)
631635
{
632636
Datum d = NumericGetDatum(n);
633-
Datum divisor_int64;
634637
Datum divisor_numeric;
635638
Datum result;
636639

637-
divisor_int64 = Int64GetDatum((int64) (1 << count));
638-
divisor_numeric = DirectFunctionCall1(int8_numeric, divisor_int64);
640+
divisor_numeric = DirectFunctionCall1(int8_numeric, divisor);
639641
result = DirectFunctionCall2(numeric_div_trunc, d, divisor_numeric);
640642
return DatumGetNumeric(result);
641643
}
@@ -658,8 +660,8 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
658660
else
659661
{
660662
/* keep one extra bit for rounding */
661-
/* size >>= 9 */
662-
size = numeric_shift_right(size, 9);
663+
/* size /= (1 << 9) */
664+
size = numeric_truncated_divide(size, 1 << 9);
663665

664666
if (numeric_is_less(numeric_absolute(size), limit2))
665667
{
@@ -668,17 +670,18 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
668670
}
669671
else
670672
{
671-
/* size >>= 10 */
672-
size = numeric_shift_right(size, 10);
673+
/* size /= (1 << 10) */
674+
size = numeric_truncated_divide(size, 1 << 10);
675+
673676
if (numeric_is_less(numeric_absolute(size), limit2))
674677
{
675678
size = numeric_half_rounded(size);
676679
result = psprintf("%s MB", numeric_to_cstring(size));
677680
}
678681
else
679682
{
680-
/* size >>= 10 */
681-
size = numeric_shift_right(size, 10);
683+
/* size /= (1 << 10) */
684+
size = numeric_truncated_divide(size, 1 << 10);
682685

683686
if (numeric_is_less(numeric_absolute(size), limit2))
684687
{
@@ -687,8 +690,8 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
687690
}
688691
else
689692
{
690-
/* size >>= 10 */
691-
size = numeric_shift_right(size, 10);
693+
/* size /= (1 << 10) */
694+
size = numeric_truncated_divide(size, 1 << 10);
692695
size = numeric_half_rounded(size);
693696
result = psprintf("%s TB", numeric_to_cstring(size));
694697
}

‎src/test/regress/expected/dbsize.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/dbsize.out
+42Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,48 @@ SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
3535
1000000000000000.5 | 909 TB | -909 TB
3636
(12 rows)
3737

38+
-- test where units change up
39+
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
40+
(VALUES (10239::bigint), (10240::bigint),
41+
(10485247::bigint), (10485248::bigint),
42+
(10736893951::bigint), (10736893952::bigint),
43+
(10994579406847::bigint), (10994579406848::bigint),
44+
(11258449312612351::bigint), (11258449312612352::bigint)) x(size);
45+
size | pg_size_pretty | pg_size_pretty
46+
-------------------+----------------+----------------
47+
10239 | 10239 bytes | -10239 bytes
48+
10240 | 10 kB | -10 kB
49+
10485247 | 10239 kB | -10239 kB
50+
10485248 | 10 MB | -10 MB
51+
10736893951 | 10239 MB | -10239 MB
52+
10736893952 | 10 GB | -10 GB
53+
10994579406847 | 10239 GB | -10239 GB
54+
10994579406848 | 10 TB | -10 TB
55+
11258449312612351 | 10239 TB | -10239 TB
56+
11258449312612352 | 10240 TB | -10240 TB
57+
(10 rows)
58+
59+
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
60+
(VALUES (10239::numeric), (10240::numeric),
61+
(10485247::numeric), (10485248::numeric),
62+
(10736893951::numeric), (10736893952::numeric),
63+
(10994579406847::numeric), (10994579406848::numeric),
64+
(11258449312612351::numeric), (11258449312612352::numeric)) x(size);
65+
size | pg_size_pretty | pg_size_pretty
66+
-------------------+----------------+----------------
67+
10239 | 10239 bytes | -10239 bytes
68+
10240 | 10 kB | -10 kB
69+
10485247 | 10239 kB | -10239 kB
70+
10485248 | 10 MB | -10 MB
71+
10736893951 | 10239 MB | -10239 MB
72+
10736893952 | 10 GB | -10 GB
73+
10994579406847 | 10239 GB | -10239 GB
74+
10994579406848 | 10 TB | -10 TB
75+
11258449312612351 | 10239 TB | -10239 TB
76+
11258449312612352 | 10240 TB | -10240 TB
77+
(10 rows)
78+
79+
-- pg_size_bytes() tests
3880
SELECT size, pg_size_bytes(size) FROM
3981
(VALUES ('1'), ('123bytes'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '),
4082
('1TB'), ('3000 TB'), ('1e6 MB')) x(size);

‎src/test/regress/sql/dbsize.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/dbsize.sql
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,22 @@ SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
1111
(1000000000.5::numeric), (1000000000000.5::numeric),
1212
(1000000000000000.5::numeric)) x(size);
1313

14+
-- test where units change up
15+
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
16+
(VALUES (10239::bigint), (10240::bigint),
17+
(10485247::bigint), (10485248::bigint),
18+
(10736893951::bigint), (10736893952::bigint),
19+
(10994579406847::bigint), (10994579406848::bigint),
20+
(11258449312612351::bigint), (11258449312612352::bigint)) x(size);
21+
22+
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
23+
(VALUES (10239::numeric), (10240::numeric),
24+
(10485247::numeric), (10485248::numeric),
25+
(10736893951::numeric), (10736893952::numeric),
26+
(10994579406847::numeric), (10994579406848::numeric),
27+
(11258449312612351::numeric), (11258449312612352::numeric)) x(size);
28+
29+
-- pg_size_bytes() tests
1430
SELECT size, pg_size_bytes(size) FROM
1531
(VALUES ('1'), ('123bytes'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '),
1632
('1TB'), ('3000 TB'), ('1e6 MB')) x(size);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.