pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17876: Function width_bucket() for float8 input returns value out of range
9+ messages / 3 participants
[nested] [flat]
* BUG #17876: Function width_bucket() for float8 input returns value out of range
@ 2023-03-29 08:11 PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
0 siblings, 1 reply; 9+ messages in thread
From: PG Bug reporting form @ 2023-03-29 08:11 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: mats@timescale.com
The following bug has been logged on the website:
Bug reference: 17876
Logged by: Mats Kindahl
Email address: mats@timescale.com
PostgreSQL version: 15.2
Operating system: All
Description:
On 64-bit platforms, the existing width_bucket() computation can
result in a NaN (0x8000000) if the difference between the `operand`,
`low`, or `high` parameters exceeds DBL_MAX. This is then cast to the
value -2147483648.
mats=# WITH sample(operand, low, high, cnt) AS (
VALUES
(10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 1::int4),
(10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 2),
(10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 3),
(1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
(-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
(1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
(-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
(1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2,
10),
(-1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2,
10)
) SELECT width_bucket(operand, low, high, cnt) FROM sample;
width_bucket
--------------
1
-2147483648
-2147483648
-2147483648
-2147483648
-2147483648
-2147483648
11
0
(9 rows)
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
@ 2023-03-29 08:18 ` Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Mats Kindahl @ 2023-03-29 08:18 UTC (permalink / raw)
To: mats@timescale.com; pgsql-bugs@lists.postgresql.org
Hi all,
Attached is a proposed fix for the issue. It has extended the tests to
cover these cases and also some additional tests for values close to
DBL_MIN. I compared the old and new version using the attached program with
the following result:
DBL_MAX: 1.797693e+308, DBL_MIN: 2.225074e-308
1.04000e+01, low: -1.79769e+308, high: 1.79769e+308, count: 10 --> orig:
-2147483648, new: 6
-8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 10 -->
orig: -2147483648, new: 3
8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 10 -->
orig: -2147483648, new: 8
1.04000e+01, low: -1.79769e+308, high: 1.79769e+308, count: 12 --> orig:
-2147483648, new: 7
-8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 12 -->
orig: -2147483648, new: 4
8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 12 -->
orig: -2147483648, new: 10
1.04000e+01, low: -1.79769e+308, high: 1.79769e+308, count: 1 --> orig:
1, new: 1
-8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 1 --> orig:
1, new: 1
8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 1 --> orig:
-2147483648, new: 1
1.04000e+01, low: -1.79769e+308, high: 1.79769e+308, count: 2 --> orig:
-2147483648, new: 2
-8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 2 --> orig:
1, new: 1
8.98847e+307, low: -1.79769e+308, high: 1.79769e+308, count: 2 --> orig:
-2147483648, new: 2
5.35000e+00, low: 2.40000e-02, high: 1.00600e+01, count: 5 --> orig:
3, new: 3
2.22507e-308, low: -4.45015e-308, high: 4.45015e-308, count: 4 --> orig:
4, new: 4
-2.22507e-308, low: -4.45015e-308, high: 4.45015e-308, count: 4 --> orig:
2, new: 2
2.22507e-308, low: -6.67522e-308, high: 6.67522e-308, count: 4 --> orig:
3, new: 3
-2.22507e-308, low: -6.67522e-308, high: 6.67522e-308, count: 4 --> orig:
2, new: 2
2.22507e-308, low: -6.67522e-308, high: 6.67522e-308, count: 6 --> orig:
5, new: 5
-2.22507e-308, low: -6.67522e-308, high: 6.67522e-308, count: 6 --> orig:
3, new: 3
As you can see, the old and new computations seem to produce the same
values also for values close to DBL_MIN.
On Wed, Mar 29, 2023 at 10:12 AM PG Bug reporting form <
noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17876
> Logged by: Mats Kindahl
> Email address: mats@timescale.com
> PostgreSQL version: 15.2
> Operating system: All
> Description:
>
> On 64-bit platforms, the existing width_bucket() computation can
> result in a NaN (0x8000000) if the difference between the `operand`,
> `low`, or `high` parameters exceeds DBL_MAX. This is then cast to the
> value -2147483648.
>
> mats=# WITH sample(operand, low, high, cnt) AS (
> VALUES
> (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 1::int4),
> (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 2),
> (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 3),
>
> (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8,
> 10),
> (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8,
> 10),
>
> (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8,
> 16),
> (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8,
> 16),
>
> (1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 /
> 2,
> 10),
> (-1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8
> / 2,
> 10)
> ) SELECT width_bucket(operand, low, high, cnt) FROM sample;
> width_bucket
> --------------
> 1
> -2147483648
> -2147483648
> -2147483648
> -2147483648
> -2147483648
> -2147483648
> 11
> 0
> (9 rows)
>
>
Attachments:
[application/x-patch] 0001-Avoid-floating-point-overflow-in-width_bucket.patch (5.9K, ../../CA+14427KvPsskajKSriGACyrudcOyh5ohmt=0WPuNjoMTSQM6A@mail.gmail.com/3-0001-Avoid-floating-point-overflow-in-width_bucket.patch)
download | inline diff:
From 9e7a5769e41d9493eb429141e827af15ad25e730 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <mats@timescale.com>
Date: Wed, 29 Mar 2023 10:07:18 +0200
Subject: [PATCH] Avoid floating-point overflow in width_bucket()
The floating-point calculation in width_bucket() can lead to overflow
when values are close to DBL_MAX and -DBL_MAX, and further result in a
NaN in the computation. This is then cast to an integer value, which
produces an out-of-bounds value from width_bucket().
This is fixed by scaling down the values to avoid that the
floating-point expressions overflow.
---
src/backend/utils/adt/float.c | 16 +++++++++--
src/test/regress/expected/numeric.out | 38 +++++++++++++++++++++++++++
src/test/regress/sql/numeric.sql | 26 ++++++++++++++++++
3 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index aa79487a11..438ab70e90 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -4117,8 +4117,19 @@ width_bucket_float8(PG_FUNCTION_ARGS)
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("integer out of range")));
}
- else
- result = ((float8) count * (operand - bound1) / (bound2 - bound1)) + 1;
+ else {
+ /*
+ * Since the expressions "operand - bound1" and "bound2 - bound1"
+ * can overflow for some combinations of values that are close to
+ * -DBL_MAX or DBL_MAX, this can lead NaN values, which when cast
+ * to integer lead to strange values.
+ *
+ * To avoid this, we divide the values by 2 before performing the
+ * subtraction. This will ensure that the expressions will not
+ * overflow and lead to a strange result.
+ */
+ result = (count * ((operand / 2 - bound1 / 2) / (bound2 / 2 - bound1 / 2))) + 1;
+ }
}
else if (bound1 > bound2)
{
@@ -4142,5 +4153,6 @@ width_bucket_float8(PG_FUNCTION_ARGS)
result = 0; /* keep the compiler quiet */
}
+ Assert(result >= 0 && result <= count + 1);
PG_RETURN_INT32(result);
}
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 26930f4db4..2191110554 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1433,6 +1433,44 @@ SELECT width_bucket('Infinity'::float8, 1, 10, 10),
11 | 0
(1 row)
+WITH sample(oper, low, high, cnt) AS (
+ VALUES
+ -- Check ranges that can cause overflow in the internal
+ -- calculations. These are basically computations where difference
+ -- between operand and low and high can exceed DBL_MAX.
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 1::int4),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 2),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 3),
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+ (1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+ (-1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+ -- Check that potential underflow situations does not result in
+ -- weird values.
+ (2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (-2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6),
+ (-2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6)
+) SELECT width_bucket(oper, low, high, cnt) FROM sample;
+ width_bucket
+--------------
+ 1
+ 2
+ 2
+ 8
+ 3
+ 12
+ 5
+ 11
+ 0
+ 4
+ 2
+ 5
+ 3
+(13 rows)
+
DROP TABLE width_bucket_test;
-- Simple test for roundoff error when results should be exact
SELECT x, width_bucket(x::float8, 10, 100, 9) as flt,
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 2dddb58625..13ae07b6ff 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -900,6 +900,32 @@ SELECT width_bucket(0.0::float8, 5, '-Infinity'::float8, 20); -- error
SELECT width_bucket('Infinity'::float8, 1, 10, 10),
width_bucket('-Infinity'::float8, 1, 10, 10);
+WITH sample(oper, low, high, cnt) AS (
+ VALUES
+ -- Check ranges that can cause overflow in the internal
+ -- calculations. These are basically computations where difference
+ -- between operand and low and high can exceed DBL_MAX.
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 1::int4),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 2),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 3),
+
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+
+ (1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+ (-1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+
+ -- Check that potential underflow situations does not result in
+ -- weird values.
+ (2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (-2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6),
+ (-2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6)
+) SELECT width_bucket(oper, low, high, cnt) FROM sample;
+
DROP TABLE width_bucket_test;
-- Simple test for roundoff error when results should be exact
--
2.34.1
[text/x-c-code] float_bucket.c (2.4K, ../../CA+14427KvPsskajKSriGACyrudcOyh5ohmt=0WPuNjoMTSQM6A@mail.gmail.com/4-float_bucket.c)
download | inline:
#include <float.h>
#include <stdio.h>
#include <stdlib.h>
/*
* The two expressions "high - low" can result in +inf if "high" and
* "low" are have a distance exceeding DBL_MAX. Similarly, the
* expression "oper - low" can be +inf if "oper" and "low" differs
* by more than DBL_MAX.
*/
int width_bucket_orig(double oper, double low, double high, int count) {
return ((double)count * (oper - low) / (high - low)) + 1;
}
int width_bucket_new(double oper, double low, double high, int count) {
const double bucket_size = (high / 2 - low / 2);
const double bucket_pos = (oper / 2 - low / 2);
return count * (bucket_pos / bucket_size) + 1;
}
static struct value {
double operand;
double low;
double high;
int count;
} values[] = {
{10.4, -DBL_MAX, DBL_MAX, 10},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 10},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 10},
{10.4, -DBL_MAX, DBL_MAX, 12},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 12},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 12},
{10.4, -DBL_MAX, DBL_MAX, 1},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 1},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 1},
{10.4, -DBL_MAX, DBL_MAX, 2},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 2},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 2},
{5.35, 0.024, 10.06, 5},
{DBL_MIN, -2 * DBL_MIN, 2 * DBL_MIN, 4},
{-DBL_MIN, -2 * DBL_MIN, 2 * DBL_MIN, 4},
{DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 4},
{-DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 4},
{DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 6},
{-DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 6},
};
void print_result(double oper, double low, double high, int count) {
printf(
"% 2.5e, low: % 2.5e, high: % 2.5e, count: % 2d --> orig: % 2d, new: % "
"2d\n",
oper,
low,
high,
count,
width_bucket_orig(oper, low, high, count),
width_bucket_new(oper, low, high, count));
}
int main(int argc, char *argv[]) {
printf("DBL_MAX: %e, DBL_MIN: %e\n", DBL_MAX, DBL_MIN);
if (argc == 1) {
int i;
for (i = 0; i < sizeof(values) / sizeof(*values); ++i)
print_result(
values[i].operand, values[i].low, values[i].high, values[i].count);
} else if (argc == 5) {
print_result(strtod(argv[1], NULL),
strtod(argv[2], NULL),
strtod(argv[3], NULL),
strtol(argv[4], NULL, 0));
} else {
fprintf(stderr, "Usage: <oper> <low> <high> <count>\n");
exit(2);
}
}
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
@ 2023-03-29 12:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2023-03-29 12:33 UTC (permalink / raw)
To: Mats Kindahl <mats@timescale.com>; +Cc: pgsql-bugs@lists.postgresql.org
Mats Kindahl <mats@timescale.com> writes:
> Attached is a proposed fix for the issue.
Hm. Aren't you replacing a risk of overflow with a risk of underflow?
I'd be happier about doing this only if isinf(bound2 - bound1), or
the reverse for the other path. (Seems like we shouldn't need to
check the operand diff separately.)
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-03-30 07:54 ` Mats Kindahl <mats@timescale.com>
2023-03-30 15:35 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Mats Kindahl @ 2023-03-30 07:54 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
On Wed, Mar 29, 2023 at 2:33 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mats Kindahl <mats@timescale.com> writes:
> > Attached is a proposed fix for the issue.
>
> Hm. Aren't you replacing a risk of overflow with a risk of underflow?
> I'd be happier about doing this only if isinf(bound2 - bound1), or
> the reverse for the other path. (Seems like we shouldn't need to
> check the operand diff separately.)
>
Yes, I was wondering the same which is why I was adding these tests as a
separate file to see if I could force a bad bucket. It seems to be hard to
construct a case where the underflow would cause a change in the result.
However, better safe than sorry, so I modified the patch to include the
check. And yes, you're right in that there is no need to check for the
operand diff since the previous checks guarantee that the operand is
between the bounds, and since the diff between the bounds is not infinite,
the diff between the operand and any of the bounds cannot be infinite.
Added a comment to that effect to the patch as well.
Best wishes,
Mats Kindahl, Timescale
> regards, tom lane
>
Attachments:
[text/x-patch] 0001-Avoid-floating-point-overflow-in-width_bucket.patch (6.3K, ../../CA+14424DZUcLpWnoRURmT1m6Uet1_R39sV=8XhtFRiigVtYqDg@mail.gmail.com/3-0001-Avoid-floating-point-overflow-in-width_bucket.patch)
download | inline diff:
From 1ee425f4dfb35b73b68e50295ee37664618a1856 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <mats@timescale.com>
Date: Wed, 29 Mar 2023 10:07:18 +0200
Subject: [PATCH] Avoid floating-point overflow in width_bucket()
The floating-point calculation in width_bucket() can lead to overflow
when values are close to DBL_MAX and -DBL_MAX, and further result in a
NaN in the computation. This is then cast to an integer value, which
produces an out-of-bounds value from width_bucket().
This is fixed by scaling down the values to avoid that the
floating-point expressions overflow.
---
src/backend/utils/adt/float.c | 21 ++++++++++++++-
src/test/regress/expected/numeric.out | 38 +++++++++++++++++++++++++++
src/test/regress/sql/numeric.sql | 26 ++++++++++++++++++
3 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index aa79487a11..39a7f1a6e4 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -4117,8 +4117,26 @@ width_bucket_float8(PG_FUNCTION_ARGS)
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("integer out of range")));
}
+ /*
+ * Since the expressions "operand - bound1" and "bound2 - bound1" can
+ * overflow for some combinations of values that are close to -DBL_MAX
+ * or DBL_MAX, this can lead NaN values, which when cast to integer
+ * lead to strange values.
+ *
+ * To avoid this, we divide the values by 2 before performing the
+ * subtraction if the result of either of them is infinite. This will
+ * ensure that the expressions will not overflow and lead to a strange
+ * result.
+ *
+ * Since we know from the previous checks that bound1 <= operand <=
+ * bound2 it is sufficient to check that bound2 - bound1 is not
+ * infinite. If it is not infinite, then operand - bound1 cannot be
+ * infinite either.
+ */
+ else if (isinf(bound2 - bound1))
+ result = (count * ((operand / 2 - bound1 / 2) / (bound2 / 2 - bound1 / 2))) + 1;
else
- result = ((float8) count * (operand - bound1) / (bound2 - bound1)) + 1;
+ result = (count * ((operand - bound1) / (bound2 - bound1))) + 1;
}
else if (bound1 > bound2)
{
@@ -4142,5 +4160,6 @@ width_bucket_float8(PG_FUNCTION_ARGS)
result = 0; /* keep the compiler quiet */
}
+ Assert(result >= 0 && result <= count + 1);
PG_RETURN_INT32(result);
}
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 26930f4db4..2191110554 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1433,6 +1433,44 @@ SELECT width_bucket('Infinity'::float8, 1, 10, 10),
11 | 0
(1 row)
+WITH sample(oper, low, high, cnt) AS (
+ VALUES
+ -- Check ranges that can cause overflow in the internal
+ -- calculations. These are basically computations where difference
+ -- between operand and low and high can exceed DBL_MAX.
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 1::int4),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 2),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 3),
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+ (1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+ (-1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+ -- Check that potential underflow situations does not result in
+ -- weird values.
+ (2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (-2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6),
+ (-2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6)
+) SELECT width_bucket(oper, low, high, cnt) FROM sample;
+ width_bucket
+--------------
+ 1
+ 2
+ 2
+ 8
+ 3
+ 12
+ 5
+ 11
+ 0
+ 4
+ 2
+ 5
+ 3
+(13 rows)
+
DROP TABLE width_bucket_test;
-- Simple test for roundoff error when results should be exact
SELECT x, width_bucket(x::float8, 10, 100, 9) as flt,
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 2dddb58625..13ae07b6ff 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -900,6 +900,32 @@ SELECT width_bucket(0.0::float8, 5, '-Infinity'::float8, 20); -- error
SELECT width_bucket('Infinity'::float8, 1, 10, 10),
width_bucket('-Infinity'::float8, 1, 10, 10);
+WITH sample(oper, low, high, cnt) AS (
+ VALUES
+ -- Check ranges that can cause overflow in the internal
+ -- calculations. These are basically computations where difference
+ -- between operand and low and high can exceed DBL_MAX.
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 1::int4),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 2),
+ (10.5::float8, -1.797e+308::float8, 1.797e+308::float8, 3),
+
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 10),
+
+ (1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+ (-1.797e+308::float8 / 2, -1.797e+308::float8, 1.797e+308::float8, 16),
+
+ (1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+ (-1.797e+308::float8, -1.797e+308::float8 / 2, 1.797e+308::float8 / 2, 10),
+
+ -- Check that potential underflow situations does not result in
+ -- weird values.
+ (2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (-2.22507e-308::float8, 2 * -2.22507e-308::float8, 2 * 2.22507e-308::float8, 4),
+ (2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6),
+ (-2.22507e-308::float8, 3 * -2.22507e-308::float8, 3 * 2.22507e-308::float8, 6)
+) SELECT width_bucket(oper, low, high, cnt) FROM sample;
+
DROP TABLE width_bucket_test;
-- Simple test for roundoff error when results should be exact
--
2.34.1
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
@ 2023-03-30 15:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-03-31 09:30 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2023-03-30 15:35 UTC (permalink / raw)
To: Mats Kindahl <mats@timescale.com>; +Cc: pgsql-bugs@lists.postgresql.org
Mats Kindahl <mats@timescale.com> writes:
> However, better safe than sorry, so I modified the patch to include the
> check. And yes, you're right in that there is no need to check for the
> operand diff since the previous checks guarantee that the operand is
> between the bounds, and since the diff between the bounds is not infinite,
> the diff between the operand and any of the bounds cannot be infinite.
> Added a comment to that effect to the patch as well.
I looked this over and noted two problems:
* You missed fixing the mirror code path (bound1 > bound2).
* It seems at least possible that, for an operand just slightly less
than bound2, the quotient ((operand - bound1) / (bound2 - bound1))
could round to exactly 1, even though it should theoretically always
be in [0, 1). If that did happen, and count is INT_MAX, then the final
addition of 1 would create its own possibility of integer overflow.
We have code to check that but it's only applied in the operand >= bound2
case. I fixed that by moving the overflow-aware addition of 1 to the
bottom of the function so it's done in all cases, and adjusting the other
code paths to account for that.
Pushed with those changes and some cosmetic tweaking.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-30 15:35 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-03-31 09:30 ` Mats Kindahl <mats@timescale.com>
2023-03-31 13:14 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Mats Kindahl @ 2023-03-31 09:30 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
On Thu, Mar 30, 2023 at 5:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mats Kindahl <mats@timescale.com> writes:
> > However, better safe than sorry, so I modified the patch to include the
> > check. And yes, you're right in that there is no need to check for the
> > operand diff since the previous checks guarantee that the operand is
> > between the bounds, and since the diff between the bounds is not
> infinite,
> > the diff between the operand and any of the bounds cannot be infinite.
> > Added a comment to that effect to the patch as well.
>
> I looked this over and noted two problems:
>
> * You missed fixing the mirror code path (bound1 > bound2).
>
> * It seems at least possible that, for an operand just slightly less
> than bound2, the quotient ((operand - bound1) / (bound2 - bound1))
> could round to exactly 1, even though it should theoretically always
> be in [0, 1). If that did happen, and count is INT_MAX, then the final
> addition of 1 would create its own possibility of integer overflow.
> We have code to check that but it's only applied in the operand >= bound2
> case. I fixed that by moving the overflow-aware addition of 1 to the
> bottom of the function so it's done in all cases, and adjusting the other
> code paths to account for that.
>
> Pushed with those changes and some cosmetic tweaking.
>
Thanks Tom!
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-30 15:35 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-31 09:30 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
@ 2023-03-31 13:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-04-05 07:58 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2023-03-31 13:14 UTC (permalink / raw)
To: Mats Kindahl <mats@timescale.com>; +Cc: pgsql-bugs@lists.postgresql.org
Mats Kindahl <mats@timescale.com> writes:
> On Thu, Mar 30, 2023 at 5:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * It seems at least possible that, for an operand just slightly less
>> than bound2, the quotient ((operand - bound1) / (bound2 - bound1))
>> could round to exactly 1, even though it should theoretically always
>> be in [0, 1). If that did happen, and count is INT_MAX, then the final
>> addition of 1 would create its own possibility of integer overflow.
>> We have code to check that but it's only applied in the operand >= bound2
>> case. I fixed that by moving the overflow-aware addition of 1 to the
>> bottom of the function so it's done in all cases, and adjusting the other
>> code paths to account for that.
I realized that it's actually not too hard to make that happen:
regression=# select width_bucket(0, -1e100::float8, 1, 10);
width_bucket
--------------
11
(1 row)
While I'm not bothered too much if rounding affects which internal
bucket a value lands in, it's a bit more annoying if that causes
it to be reported as being in the end bucket, when we know positively
that the value is less than bound2. Is it worth expending more
cycles to prevent this? It'd need to be something like
/* Result of division is surely in [0,1], so this can't overflow */
result = count * ((operand - bound1) / (bound2 - bound1));
+ /* ... but the quotient could round to 1, which would be a lie */
+ if (result >= count)
+ result = count - 1;
and we'd need two or four copies of that depending on whether we want
to refactor some more.
Curiously, width_bucket_numeric has this problem too:
regression=# select width_bucket(0, -1e100::numeric, 1, 10);
width_bucket
--------------
11
(1 row)
I suppose it's also rounding somewhere in there.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-30 15:35 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-31 09:30 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-31 13:14 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-04-05 07:58 ` Mats Kindahl <mats@timescale.com>
2023-04-05 21:41 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Mats Kindahl @ 2023-04-05 07:58 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
On Fri, Mar 31, 2023 at 3:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mats Kindahl <mats@timescale.com> writes:
> > On Thu, Mar 30, 2023 at 5:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> * It seems at least possible that, for an operand just slightly less
> >> than bound2, the quotient ((operand - bound1) / (bound2 - bound1))
> >> could round to exactly 1, even though it should theoretically always
> >> be in [0, 1). If that did happen, and count is INT_MAX, then the final
> >> addition of 1 would create its own possibility of integer overflow.
> >> We have code to check that but it's only applied in the operand >=
> bound2
> >> case. I fixed that by moving the overflow-aware addition of 1 to the
> >> bottom of the function so it's done in all cases, and adjusting the
> other
> >> code paths to account for that.
>
> I realized that it's actually not too hard to make that happen:
>
> regression=# select width_bucket(0, -1e100::float8, 1, 10);
> width_bucket
> --------------
> 11
> (1 row)
>
> While I'm not bothered too much if rounding affects which internal
> bucket a value lands in, it's a bit more annoying if that causes
> it to be reported as being in the end bucket, when we know positively
> that the value is less than bound2. Is it worth expending more
> cycles to prevent this?
I think it could be. I agree that it does not make sense to select the end
bucket when the value is in range, even if just.
> It'd need to be something like
>
> /* Result of division is surely in [0,1], so this can't
> overflow */
> result = count * ((operand - bound1) / (bound2 - bound1));
> + /* ... but the quotient could round to 1, which would be a lie
> */
> + if (result >= count)
> + result = count - 1;
>
Just doing some quick measurements on the different alternatives, I do not
see a big difference in performance between doing this additional check.
Have a look at the attached files. You can probably add a few other
alternatives to see if it can be improved further. Compiled at -O2.
>
> and we'd need two or four copies of that depending on whether we want
> to refactor some more.
>
> Curiously, width_bucket_numeric has this problem too:
>
> regression=# select width_bucket(0, -1e100::numeric, 1, 10);
> width_bucket
> --------------
> 11
> (1 row)
>
> I suppose it's also rounding somewhere in there.
>
Best wishes,
Mats Kindahl
>
> regards, tom lane
>
orig: 192.728943
null: 190.671674
new1: 214.226611
new2: 193.675008
new3: 191.269468
orig: 192.195517
null: 192.133568
new1: 215.438432
new2: 192.882215
new3: 191.575396
orig: 191.066321
null: 190.991864
new1: 212.328873
new2: 191.037750
new3: 191.205410
orig: 191.904597
null: 191.971803
new1: 212.657828
new2: 191.484456
new3: 191.276174
orig: 191.096532
null: 191.757063
new1: 212.873912
new2: 190.390716
new3: 191.181580
orig: 191.365715
null: 193.142040
new1: 212.618695
new2: 192.348662
new3: 191.690640
orig: 191.194124
null: 191.298014
new1: 212.781822
new2: 191.477476
new3: 192.386847
orig: 191.538564
null: 191.874644
new1: 212.887520
new2: 191.386105
new3: 192.091771
orig: 191.683138
null: 191.768176
new1: 213.331004
new2: 191.680956
new3: 192.846726
orig: 191.449813
null: 191.883779
new1: 213.134165
new2: 191.998603
new3: 192.581382
orig: 197.781991
null: 191.906687
new1: 225.843891
new2: 192.608844
new3: 191.367105
orig: 191.653958
null: 192.802103
new1: 213.270162
new2: 192.207371
new3: 193.217588
orig: 191.962450
null: 193.263021
new1: 214.459158
new2: 192.608462
new3: 192.999069
orig: 191.963799
null: 192.567739
new1: 213.293353
new2: 192.421379
new3: 192.573796
orig: 192.724602
null: 192.532097
new1: 213.233913
new2: 192.108951
new3: 192.790473
orig: 192.416546
null: 192.528082
new1: 213.311503
new2: 191.524358
new3: 193.233970
orig: 192.274027
null: 192.668405
new1: 213.605453
new2: 192.279015
new3: 192.664595
orig: 192.378458
null: 192.563897
new1: 213.555924
new2: 193.091948
new3: 192.530735
orig: 191.742279
null: 192.373103
new1: 213.847394
new2: 192.512144
new3: 193.060242
orig: 192.916049
null: 192.516667
new1: 213.689700
new2: 193.792995
new3: 193.080553
orig: 191.697616
null: 192.541433
new1: 214.196983
new2: 192.811939
new3: 192.390346
orig: 194.059748
null: 193.021589
new1: 214.313655
new2: 193.245450
new3: 193.659744
orig: 193.972115
null: 194.020957
new1: 215.816783
new2: 193.784965
new3: 194.352708
orig: 193.786892
null: 194.371744
new1: 215.392015
new2: 194.151938
new3: 193.360102
orig: 192.452764
null: 193.205724
new1: 214.001721
new2: 193.118897
new3: 192.449212
orig: 193.328985
null: 193.074785
new1: 214.276437
new2: 192.936366
new3: 193.211706
orig: 193.489367
null: 193.645585
new1: 214.231920
new2: 192.420045
new3: 192.967615
orig: 192.741103
null: 191.825252
new1: 214.001862
new2: 192.008872
new3: 193.456918
orig: 192.081999
null: 192.706654
new1: 214.153062
new2: 193.369150
new3: 193.042905
orig: 192.320135
null: 193.351747
new1: 214.045844
new2: 192.127948
new3: 193.540298
orig: 192.988422
null: 192.944924
new1: 214.148765
new2: 193.443620
new3: 192.752107
orig: 192.995000
null: 194.001661
new1: 214.286778
new2: 194.251692
new3: 192.618977
orig: 192.814254
null: 194.695821
new1: 214.310552
new2: 193.855163
new3: 195.014678
orig: 192.490449
null: 193.304302
new1: 214.797132
new2: 194.235828
new3: 193.796948
orig: 193.601320
null: 194.148963
new1: 214.383353
new2: 194.382489
new3: 193.723876
orig: 194.166343
null: 194.064106
new1: 214.261519
new2: 193.687962
new3: 193.151846
orig: 193.342246
null: 193.371053
new1: 214.510438
new2: 193.787092
new3: 193.055634
orig: 193.436481
null: 193.019854
new1: 214.177953
new2: 193.635735
new3: 193.683768
orig: 192.902244
null: 194.367386
new1: 213.746209
new2: 192.834811
new3: 192.551276
orig: 192.654471
null: 193.041416
new1: 213.721639
new2: 192.808550
new3: 192.330366
orig: 193.124752
null: 193.184217
new1: 213.609735
new2: 193.327366
new3: 193.451408
orig: 191.554679
null: 192.583560
new1: 213.403821
new2: 192.929166
new3: 193.532892
orig: 193.641022
null: 192.774439
new1: 213.659201
new2: 193.481640
new3: 192.567532
orig: 192.628250
null: 192.640086
new1: 213.372064
new2: 191.716284
new3: 192.055761
orig: 192.556423
null: 192.560258
new1: 213.486611
new2: 192.531515
new3: 192.797195
orig: 193.664687
null: 193.466087
new1: 213.329556
new2: 192.308785
new3: 191.653663
orig: 193.142872
null: 192.919313
new1: 213.584296
new2: 192.365876
new3: 194.315051
orig: 194.949973
null: 194.397169
new1: 216.593304
new2: 203.885696
new3: 196.339263
orig: 195.196222
null: 196.038223
new1: 223.370806
new2: 195.096831
new3: 195.064386
orig: 194.313923
null: 196.162283
new1: 216.899799
new2: 196.439525
new3: 195.136126
Attachments:
[text/plain] float_bucket_measure.txt (8.1K, ../../CA+14427tOx9GXw2Va_W0HF7Wp7kiu31BNju_=3SnCuaXOAdJzQ@mail.gmail.com/3-float_bucket_measure.txt)
download | inline:
orig: 192.728943
null: 190.671674
new1: 214.226611
new2: 193.675008
new3: 191.269468
orig: 192.195517
null: 192.133568
new1: 215.438432
new2: 192.882215
new3: 191.575396
orig: 191.066321
null: 190.991864
new1: 212.328873
new2: 191.037750
new3: 191.205410
orig: 191.904597
null: 191.971803
new1: 212.657828
new2: 191.484456
new3: 191.276174
orig: 191.096532
null: 191.757063
new1: 212.873912
new2: 190.390716
new3: 191.181580
orig: 191.365715
null: 193.142040
new1: 212.618695
new2: 192.348662
new3: 191.690640
orig: 191.194124
null: 191.298014
new1: 212.781822
new2: 191.477476
new3: 192.386847
orig: 191.538564
null: 191.874644
new1: 212.887520
new2: 191.386105
new3: 192.091771
orig: 191.683138
null: 191.768176
new1: 213.331004
new2: 191.680956
new3: 192.846726
orig: 191.449813
null: 191.883779
new1: 213.134165
new2: 191.998603
new3: 192.581382
orig: 197.781991
null: 191.906687
new1: 225.843891
new2: 192.608844
new3: 191.367105
orig: 191.653958
null: 192.802103
new1: 213.270162
new2: 192.207371
new3: 193.217588
orig: 191.962450
null: 193.263021
new1: 214.459158
new2: 192.608462
new3: 192.999069
orig: 191.963799
null: 192.567739
new1: 213.293353
new2: 192.421379
new3: 192.573796
orig: 192.724602
null: 192.532097
new1: 213.233913
new2: 192.108951
new3: 192.790473
orig: 192.416546
null: 192.528082
new1: 213.311503
new2: 191.524358
new3: 193.233970
orig: 192.274027
null: 192.668405
new1: 213.605453
new2: 192.279015
new3: 192.664595
orig: 192.378458
null: 192.563897
new1: 213.555924
new2: 193.091948
new3: 192.530735
orig: 191.742279
null: 192.373103
new1: 213.847394
new2: 192.512144
new3: 193.060242
orig: 192.916049
null: 192.516667
new1: 213.689700
new2: 193.792995
new3: 193.080553
orig: 191.697616
null: 192.541433
new1: 214.196983
new2: 192.811939
new3: 192.390346
orig: 194.059748
null: 193.021589
new1: 214.313655
new2: 193.245450
new3: 193.659744
orig: 193.972115
null: 194.020957
new1: 215.816783
new2: 193.784965
new3: 194.352708
orig: 193.786892
null: 194.371744
new1: 215.392015
new2: 194.151938
new3: 193.360102
orig: 192.452764
null: 193.205724
new1: 214.001721
new2: 193.118897
new3: 192.449212
orig: 193.328985
null: 193.074785
new1: 214.276437
new2: 192.936366
new3: 193.211706
orig: 193.489367
null: 193.645585
new1: 214.231920
new2: 192.420045
new3: 192.967615
orig: 192.741103
null: 191.825252
new1: 214.001862
new2: 192.008872
new3: 193.456918
orig: 192.081999
null: 192.706654
new1: 214.153062
new2: 193.369150
new3: 193.042905
orig: 192.320135
null: 193.351747
new1: 214.045844
new2: 192.127948
new3: 193.540298
orig: 192.988422
null: 192.944924
new1: 214.148765
new2: 193.443620
new3: 192.752107
orig: 192.995000
null: 194.001661
new1: 214.286778
new2: 194.251692
new3: 192.618977
orig: 192.814254
null: 194.695821
new1: 214.310552
new2: 193.855163
new3: 195.014678
orig: 192.490449
null: 193.304302
new1: 214.797132
new2: 194.235828
new3: 193.796948
orig: 193.601320
null: 194.148963
new1: 214.383353
new2: 194.382489
new3: 193.723876
orig: 194.166343
null: 194.064106
new1: 214.261519
new2: 193.687962
new3: 193.151846
orig: 193.342246
null: 193.371053
new1: 214.510438
new2: 193.787092
new3: 193.055634
orig: 193.436481
null: 193.019854
new1: 214.177953
new2: 193.635735
new3: 193.683768
orig: 192.902244
null: 194.367386
new1: 213.746209
new2: 192.834811
new3: 192.551276
orig: 192.654471
null: 193.041416
new1: 213.721639
new2: 192.808550
new3: 192.330366
orig: 193.124752
null: 193.184217
new1: 213.609735
new2: 193.327366
new3: 193.451408
orig: 191.554679
null: 192.583560
new1: 213.403821
new2: 192.929166
new3: 193.532892
orig: 193.641022
null: 192.774439
new1: 213.659201
new2: 193.481640
new3: 192.567532
orig: 192.628250
null: 192.640086
new1: 213.372064
new2: 191.716284
new3: 192.055761
orig: 192.556423
null: 192.560258
new1: 213.486611
new2: 192.531515
new3: 192.797195
orig: 193.664687
null: 193.466087
new1: 213.329556
new2: 192.308785
new3: 191.653663
orig: 193.142872
null: 192.919313
new1: 213.584296
new2: 192.365876
new3: 194.315051
orig: 194.949973
null: 194.397169
new1: 216.593304
new2: 203.885696
new3: 196.339263
orig: 195.196222
null: 196.038223
new1: 223.370806
new2: 195.096831
new3: 195.064386
orig: 194.313923
null: 196.162283
new1: 216.899799
new2: 196.439525
new3: 195.136126
[text/x-csrc] float_bucket.c (4.2K, ../../CA+14427tOx9GXw2Va_W0HF7Wp7kiu31BNju_=3SnCuaXOAdJzQ@mail.gmail.com/4-float_bucket.c)
download | inline:
#define _POSIX_C_SOURCE 199309L
#define _XOPEN_SOURCE 500
#include <float.h>
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
/*
* The two expressions "high - low" can result in +inf if "high" and
* "low" are have a distance exceeding DBL_MAX. Similarly, the
* expression "oper - low" can be +inf if "oper" and "low" differs
* by more than DBL_MAX.
*/
int width_bucket_orig(double oper, double low, double high, int count) {
return ((double)count * (oper - low) / (high - low)) + 1;
}
int width_bucket_null(double oper, double low, double high, int count) {
return 1;
}
int width_bucket_new1(double oper, double low, double high, int count) {
if (isinf(high - low)) {
const double bucket_size = (high / 2 - low / 2);
const double bucket_pos = (oper / 2 - low / 2);
return count * (bucket_pos / bucket_size) + 1;
} else {
return count * ((oper - low) / (high - low)) + 1;
}
}
int width_bucket_new2(double oper, double low, double high, int count) {
if (isinf(high - low)) {
high /= 2;
low /= 2;
oper /= 2;
}
return count * ((oper - low) / (high - low)) + 1;
}
int width_bucket_new3(double oper, double low, double high, int count) {
if (isinf(high - low)) {
high /= 2;
low /= 2;
oper /= 2;
}
int result = count * ((oper - low) / (high - low));
if (result == count && oper > high)
result += 1;
return result;
}
static struct value {
double operand;
double low;
double high;
int count;
} values[] = {
{10.4, -DBL_MAX, DBL_MAX, 10},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 10},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 10},
{10.4, -DBL_MAX, DBL_MAX, 12},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 12},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 12},
{10.4, -DBL_MAX, DBL_MAX, 1},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 1},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 1},
{10.4, -DBL_MAX, DBL_MAX, 2},
{-DBL_MAX / 2, -DBL_MAX, DBL_MAX, 2},
{DBL_MAX / 2, -DBL_MAX, DBL_MAX, 2},
{5.35, 0.024, 10.06, 5},
{DBL_MIN, -2 * DBL_MIN, 2 * DBL_MIN, 4},
{-DBL_MIN, -2 * DBL_MIN, 2 * DBL_MIN, 4},
{DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 4},
{-DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 4},
{DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 6},
{-DBL_MIN, -3 * DBL_MIN, 3 * DBL_MIN, 6},
{0, -1e100, 1, 10},
};
void print_result(double oper, double low, double high, int count) {
printf(
"width_bucket(% 2.5e, % 2.5e, % 2.5e, % 2d): orig: % 2d, new: % 2d, "
"new2: % 2d\n",
oper,
low,
high,
count,
width_bucket_orig(oper, low, high, count),
width_bucket_new1(oper, low, high, count),
width_bucket_new3(oper, low, high, count));
}
int measure(const char *name,
int (*width_bucket)(double, double, double, int)) {
struct timespec before, after;
clock_gettime(CLOCK_REALTIME, &before);
unsigned long sum = 0; /* To avoid optimizations removing code */
for (int i = 0; i < 10000000; ++i)
sum += width_bucket(drand48(), drand48(), drand48(), random() % 10);
clock_gettime(CLOCK_REALTIME, &after);
printf("%20s: %f\n",
name,
1000 * (after.tv_sec - before.tv_sec) +
(after.tv_nsec - before.tv_nsec) / 1e6);
return sum;
}
int main(int argc, char *argv[]) {
if (argc == 1) {
int i;
printf("DBL_MAX: %e, DBL_MIN: %e\n", DBL_MAX, DBL_MIN);
printf("2 * DBL_MAX: %e\n", 2 * DBL_MAX);
for (i = 0; i < sizeof(values) / sizeof(*values); ++i)
print_result(
values[i].operand, values[i].low, values[i].high, values[i].count);
} else if (argc == 5) {
printf("DBL_MAX: %e, DBL_MIN: %e\n", DBL_MAX, DBL_MIN);
printf("2 * DBL_MAX: %e\n", 2 * DBL_MAX);
print_result(strtod(argv[1], NULL),
strtod(argv[2], NULL),
strtod(argv[3], NULL),
strtol(argv[4], NULL, 0));
} else if (strcmp(argv[1], "--benchmark") == 0) {
long count = argc > 2 ? strtol(argv[2], NULL, 0) : 20;
for (int i = 0; i < count; ++i) {
measure("orig", width_bucket_orig);
measure("null", width_bucket_null);
measure("new1", width_bucket_new1);
measure("new2", width_bucket_new2);
measure("new3", width_bucket_new3);
}
} else {
fprintf(stderr, "Usage: <oper> <low> <high> <count>\n");
exit(2);
}
}
[application/x-perl] float_bucket_plot.pl (374B, ../../CA+14427tOx9GXw2Va_W0HF7Wp7kiu31BNju_=3SnCuaXOAdJzQ@mail.gmail.com/5-float_bucket_plot.pl)
download
[image/png] float_bucket_measure.png (32.4K, ../../CA+14427tOx9GXw2Va_W0HF7Wp7kiu31BNju_=3SnCuaXOAdJzQ@mail.gmail.com/6-float_bucket_measure.png)
download | view image
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #17876: Function width_bucket() for float8 input returns value out of range
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-30 15:35 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-03-31 09:30 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
2023-03-31 13:14 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Tom Lane <tgl@sss.pgh.pa.us>
2023-04-05 07:58 ` Re: BUG #17876: Function width_bucket() for float8 input returns value out of range Mats Kindahl <mats@timescale.com>
@ 2023-04-05 21:41 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 9+ messages in thread
From: Tom Lane @ 2023-04-05 21:41 UTC (permalink / raw)
To: Mats Kindahl <mats@timescale.com>; +Cc: pgsql-bugs@lists.postgresql.org
Mats Kindahl <mats@timescale.com> writes:
> On Fri, Mar 31, 2023 at 3:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> While I'm not bothered too much if rounding affects which internal
>> bucket a value lands in, it's a bit more annoying if that causes
>> it to be reported as being in the end bucket, when we know positively
>> that the value is less than bound2. Is it worth expending more
>> cycles to prevent this?
> I think it could be. I agree that it does not make sense to select the end
> bucket when the value is in range, even if just.
Yeah. I did that already at a2a0c7c29, sorry for not circling back
to this thread.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2023-04-05 21:41 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 08:11 BUG #17876: Function width_bucket() for float8 input returns value out of range PG Bug reporting form <noreply@postgresql.org>
2023-03-29 08:18 ` Mats Kindahl <mats@timescale.com>
2023-03-29 12:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-03-30 07:54 ` Mats Kindahl <mats@timescale.com>
2023-03-30 15:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-03-31 09:30 ` Mats Kindahl <mats@timescale.com>
2023-03-31 13:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-04-05 07:58 ` Mats Kindahl <mats@timescale.com>
2023-04-05 21:41 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox