mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-08-30 17:37:20 +02:00
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 8a1fab36a
. 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.
This commit is contained in:
parent
387925893e
commit
55fe609387
@ -31,8 +31,8 @@
|
|||||||
#include "utils/relmapper.h"
|
#include "utils/relmapper.h"
|
||||||
#include "utils/syscache.h"
|
#include "utils/syscache.h"
|
||||||
|
|
||||||
/* Divide by two and round towards positive infinity. */
|
/* Divide by two and round away from zero */
|
||||||
#define half_rounded(x) (((x) + ((x) < 0 ? 0 : 1)) / 2)
|
#define half_rounded(x) (((x) + ((x) < 0 ? -1 : 1)) / 2)
|
||||||
|
|
||||||
/* Return physical size of directory contents, or 0 if dir doesn't exist */
|
/* Return physical size of directory contents, or 0 if dir doesn't exist */
|
||||||
static int64
|
static int64
|
||||||
@ -542,25 +542,29 @@ pg_size_pretty(PG_FUNCTION_ARGS)
|
|||||||
snprintf(buf, sizeof(buf), INT64_FORMAT " bytes", size);
|
snprintf(buf, sizeof(buf), INT64_FORMAT " bytes", size);
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
size >>= 9; /* keep one extra bit for rounding */
|
/*
|
||||||
|
* We use divide instead of bit shifting so that behavior matches for
|
||||||
|
* both positive and negative size values.
|
||||||
|
*/
|
||||||
|
size /= (1 << 9); /* keep one extra bit for rounding */
|
||||||
if (Abs(size) < limit2)
|
if (Abs(size) < limit2)
|
||||||
snprintf(buf, sizeof(buf), INT64_FORMAT " kB",
|
snprintf(buf, sizeof(buf), INT64_FORMAT " kB",
|
||||||
half_rounded(size));
|
half_rounded(size));
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
size >>= 10;
|
size /= (1 << 10);
|
||||||
if (Abs(size) < limit2)
|
if (Abs(size) < limit2)
|
||||||
snprintf(buf, sizeof(buf), INT64_FORMAT " MB",
|
snprintf(buf, sizeof(buf), INT64_FORMAT " MB",
|
||||||
half_rounded(size));
|
half_rounded(size));
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
size >>= 10;
|
size /= (1 << 10);
|
||||||
if (Abs(size) < limit2)
|
if (Abs(size) < limit2)
|
||||||
snprintf(buf, sizeof(buf), INT64_FORMAT " GB",
|
snprintf(buf, sizeof(buf), INT64_FORMAT " GB",
|
||||||
half_rounded(size));
|
half_rounded(size));
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
size >>= 10;
|
size /= (1 << 10);
|
||||||
snprintf(buf, sizeof(buf), INT64_FORMAT " TB",
|
snprintf(buf, sizeof(buf), INT64_FORMAT " TB",
|
||||||
half_rounded(size));
|
half_rounded(size));
|
||||||
}
|
}
|
||||||
@ -621,13 +625,13 @@ numeric_half_rounded(Numeric n)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static Numeric
|
static Numeric
|
||||||
numeric_shift_right(Numeric n, unsigned count)
|
numeric_truncated_divide(Numeric n, int64 divisor)
|
||||||
{
|
{
|
||||||
Datum d = NumericGetDatum(n);
|
Datum d = NumericGetDatum(n);
|
||||||
Datum divisor_numeric;
|
Datum divisor_numeric;
|
||||||
Datum result;
|
Datum result;
|
||||||
|
|
||||||
divisor_numeric = NumericGetDatum(int64_to_numeric(((int64) 1) << count));
|
divisor_numeric = NumericGetDatum(int64_to_numeric(divisor));
|
||||||
result = DirectFunctionCall2(numeric_div_trunc, d, divisor_numeric);
|
result = DirectFunctionCall2(numeric_div_trunc, d, divisor_numeric);
|
||||||
return DatumGetNumeric(result);
|
return DatumGetNumeric(result);
|
||||||
}
|
}
|
||||||
@ -650,8 +654,8 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
|
|||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* keep one extra bit for rounding */
|
/* keep one extra bit for rounding */
|
||||||
/* size >>= 9 */
|
/* size /= (1 << 9) */
|
||||||
size = numeric_shift_right(size, 9);
|
size = numeric_truncated_divide(size, 1 << 9);
|
||||||
|
|
||||||
if (numeric_is_less(numeric_absolute(size), limit2))
|
if (numeric_is_less(numeric_absolute(size), limit2))
|
||||||
{
|
{
|
||||||
@ -660,8 +664,9 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* size >>= 10 */
|
/* size /= (1 << 10) */
|
||||||
size = numeric_shift_right(size, 10);
|
size = numeric_truncated_divide(size, 1 << 10);
|
||||||
|
|
||||||
if (numeric_is_less(numeric_absolute(size), limit2))
|
if (numeric_is_less(numeric_absolute(size), limit2))
|
||||||
{
|
{
|
||||||
size = numeric_half_rounded(size);
|
size = numeric_half_rounded(size);
|
||||||
@ -669,8 +674,8 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* size >>= 10 */
|
/* size /= (1 << 10) */
|
||||||
size = numeric_shift_right(size, 10);
|
size = numeric_truncated_divide(size, 1 << 10);
|
||||||
|
|
||||||
if (numeric_is_less(numeric_absolute(size), limit2))
|
if (numeric_is_less(numeric_absolute(size), limit2))
|
||||||
{
|
{
|
||||||
@ -679,8 +684,8 @@ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* size >>= 10 */
|
/* size /= (1 << 10) */
|
||||||
size = numeric_shift_right(size, 10);
|
size = numeric_truncated_divide(size, 1 << 10);
|
||||||
size = numeric_half_rounded(size);
|
size = numeric_half_rounded(size);
|
||||||
result = psprintf("%s TB", numeric_to_cstring(size));
|
result = psprintf("%s TB", numeric_to_cstring(size));
|
||||||
}
|
}
|
||||||
|
@ -35,6 +35,48 @@ SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
|
|||||||
1000000000000000.5 | 909 TB | -909 TB
|
1000000000000000.5 | 909 TB | -909 TB
|
||||||
(12 rows)
|
(12 rows)
|
||||||
|
|
||||||
|
-- test where units change up
|
||||||
|
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
|
||||||
|
(VALUES (10239::bigint), (10240::bigint),
|
||||||
|
(10485247::bigint), (10485248::bigint),
|
||||||
|
(10736893951::bigint), (10736893952::bigint),
|
||||||
|
(10994579406847::bigint), (10994579406848::bigint),
|
||||||
|
(11258449312612351::bigint), (11258449312612352::bigint)) x(size);
|
||||||
|
size | pg_size_pretty | pg_size_pretty
|
||||||
|
-------------------+----------------+----------------
|
||||||
|
10239 | 10239 bytes | -10239 bytes
|
||||||
|
10240 | 10 kB | -10 kB
|
||||||
|
10485247 | 10239 kB | -10239 kB
|
||||||
|
10485248 | 10 MB | -10 MB
|
||||||
|
10736893951 | 10239 MB | -10239 MB
|
||||||
|
10736893952 | 10 GB | -10 GB
|
||||||
|
10994579406847 | 10239 GB | -10239 GB
|
||||||
|
10994579406848 | 10 TB | -10 TB
|
||||||
|
11258449312612351 | 10239 TB | -10239 TB
|
||||||
|
11258449312612352 | 10240 TB | -10240 TB
|
||||||
|
(10 rows)
|
||||||
|
|
||||||
|
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
|
||||||
|
(VALUES (10239::numeric), (10240::numeric),
|
||||||
|
(10485247::numeric), (10485248::numeric),
|
||||||
|
(10736893951::numeric), (10736893952::numeric),
|
||||||
|
(10994579406847::numeric), (10994579406848::numeric),
|
||||||
|
(11258449312612351::numeric), (11258449312612352::numeric)) x(size);
|
||||||
|
size | pg_size_pretty | pg_size_pretty
|
||||||
|
-------------------+----------------+----------------
|
||||||
|
10239 | 10239 bytes | -10239 bytes
|
||||||
|
10240 | 10 kB | -10 kB
|
||||||
|
10485247 | 10239 kB | -10239 kB
|
||||||
|
10485248 | 10 MB | -10 MB
|
||||||
|
10736893951 | 10239 MB | -10239 MB
|
||||||
|
10736893952 | 10 GB | -10 GB
|
||||||
|
10994579406847 | 10239 GB | -10239 GB
|
||||||
|
10994579406848 | 10 TB | -10 TB
|
||||||
|
11258449312612351 | 10239 TB | -10239 TB
|
||||||
|
11258449312612352 | 10240 TB | -10240 TB
|
||||||
|
(10 rows)
|
||||||
|
|
||||||
|
-- pg_size_bytes() tests
|
||||||
SELECT size, pg_size_bytes(size) FROM
|
SELECT size, pg_size_bytes(size) FROM
|
||||||
(VALUES ('1'), ('123bytes'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '),
|
(VALUES ('1'), ('123bytes'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '),
|
||||||
('1TB'), ('3000 TB'), ('1e6 MB')) x(size);
|
('1TB'), ('3000 TB'), ('1e6 MB')) x(size);
|
||||||
|
@ -11,6 +11,22 @@ SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
|
|||||||
(1000000000.5::numeric), (1000000000000.5::numeric),
|
(1000000000.5::numeric), (1000000000000.5::numeric),
|
||||||
(1000000000000000.5::numeric)) x(size);
|
(1000000000000000.5::numeric)) x(size);
|
||||||
|
|
||||||
|
-- test where units change up
|
||||||
|
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
|
||||||
|
(VALUES (10239::bigint), (10240::bigint),
|
||||||
|
(10485247::bigint), (10485248::bigint),
|
||||||
|
(10736893951::bigint), (10736893952::bigint),
|
||||||
|
(10994579406847::bigint), (10994579406848::bigint),
|
||||||
|
(11258449312612351::bigint), (11258449312612352::bigint)) x(size);
|
||||||
|
|
||||||
|
SELECT size, pg_size_pretty(size), pg_size_pretty(-1 * size) FROM
|
||||||
|
(VALUES (10239::numeric), (10240::numeric),
|
||||||
|
(10485247::numeric), (10485248::numeric),
|
||||||
|
(10736893951::numeric), (10736893952::numeric),
|
||||||
|
(10994579406847::numeric), (10994579406848::numeric),
|
||||||
|
(11258449312612351::numeric), (11258449312612352::numeric)) x(size);
|
||||||
|
|
||||||
|
-- pg_size_bytes() tests
|
||||||
SELECT size, pg_size_bytes(size) FROM
|
SELECT size, pg_size_bytes(size) FROM
|
||||||
(VALUES ('1'), ('123bytes'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '),
|
(VALUES ('1'), ('123bytes'), ('1kB'), ('1MB'), (' 1 GB'), ('1.5 GB '),
|
||||||
('1TB'), ('3000 TB'), ('1e6 MB')) x(size);
|
('1TB'), ('3000 TB'), ('1e6 MB')) x(size);
|
||||||
|
Loading…
Reference in New Issue
Block a user