Here are two patches.
The 0001-*.patch simply adds a regression test to numeric.sql that bits
aren't lost casting from float[48] to numeric. Naturally, it fails at this
stage.
The 0002-*.patch is a proof-of-concept patching float4_numeric and
float8_numeric in the trivial way (just using FLT_DECIMAL_DIG and
DBL_DECIMAL_DIG in place of FLT_DIG and DBL_DIG). It makes the new
regression test pass. (It will only work under a compiler that has
__FLT_DECIMAL_DIG__ and __DBL_DECIMAL_DIG__ available, and I used
those internal versions to avoid mucking with build tooling to change
the target C standard, which I assume wouldn't be welcome anyway.
This patch is just POC and not proposed as anything else.)
It changes the output of an existing numeric test and a few numeric
aggregate tests.
I have adjusted the numeric test: its purpose is to check the direction
of numeric rounding of ties, but the value to be rounded was computed
in float8 and then cast to numeric, and because negative powers of ten
aren't tidy in binary, it can turn out that the float8 computation
produces a correctly-rounded-53-bit-result that is on the nearer-to-zero
side of a tie, and now that that result is correctly cast, the resulting
numeric doesn't round in the away-from-zero direction.
I changed that test because I concluded it wasn't meant to test
float8-to-numeric casting, but only the rounding of tied numerics,
so I just made the inner expression be typed numeric, and the expected
output is unchanged.
The three aggregate tests with changed output are working from a table
of float4 values, and my assumption is they are now simply producing
the correct aggregate results given the full precision of the input
values, but I haven't confirmed that yet, so this patch leaves those
three failing for now.
-Chap
>From 5b3b05009830b29d1d3167bcd3321228535161dc Mon Sep 17 00:00:00 2001
From: Chapman Flack <c...@anastigmatix.net>
Date: Sun, 25 Feb 2018 22:52:22 -0500
Subject: [PATCH 1/2] Add regression test for cast float[48] to numeric.
At this stage, as expected, it fails.
---
src/test/regress/expected/numeric.out | 34 ++++++++++++++++++++++++++++++++++
src/test/regress/sql/numeric.sql | 31 +++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 17985e8..c186385 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -2063,3 +2063,37 @@ SELECT SUM((-9999)::numeric) FROM generate_series(1, 100000);
-999900000
(1 row)
+--
+-- Tests for cast from float4, float8
+--
+-- check precision of mantissa not lost
+WITH RECURSIVE
+ f4bit(place) AS (
+ VALUES (1::float4)
+ UNION
+ SELECT place/2::float4
+ FROM f4bit
+ WHERE 1::float4 + place/2::float4 > 1::float4
+ ),
+ f4(epsilon) AS (select min(place) FROM f4bit),
+ f8bit(place) AS (
+ VALUES (1::float8)
+ UNION
+ SELECT place/2::float8
+ FROM f8bit
+ WHERE 1::float8 + place/2::float8 > 1::float8
+ ),
+ f8(epsilon) AS (SELECT min(place) FROM f8bit),
+ testvalues(f4, f8) AS (
+ SELECT 1::float4+f4.epsilon, 1::float8+f8.epsilon
+ FROM f4, f8
+ )
+SELECT
+ f4::numeric > 1::numeric AS f4numericpreserved,
+ f8::numeric > 1::numeric AS f8numericpreserved
+FROM testvalues;
+ f4numericpreserved | f8numericpreserved
+--------------------+--------------------
+ t | t
+(1 row)
+
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index d77504e..dab9ae2 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -1036,3 +1036,34 @@ select scale(-13.000000000000000);
-- cases that need carry propagation
SELECT SUM(9999::numeric) FROM generate_series(1, 100000);
SELECT SUM((-9999)::numeric) FROM generate_series(1, 100000);
+
+--
+-- Tests for cast from float4, float8
+--
+
+-- check precision of mantissa not lost
+WITH RECURSIVE
+ f4bit(place) AS (
+ VALUES (1::float4)
+ UNION
+ SELECT place/2::float4
+ FROM f4bit
+ WHERE 1::float4 + place/2::float4 > 1::float4
+ ),
+ f4(epsilon) AS (select min(place) FROM f4bit),
+ f8bit(place) AS (
+ VALUES (1::float8)
+ UNION
+ SELECT place/2::float8
+ FROM f8bit
+ WHERE 1::float8 + place/2::float8 > 1::float8
+ ),
+ f8(epsilon) AS (SELECT min(place) FROM f8bit),
+ testvalues(f4, f8) AS (
+ SELECT 1::float4+f4.epsilon, 1::float8+f8.epsilon
+ FROM f4, f8
+ )
+SELECT
+ f4::numeric > 1::numeric AS f4numericpreserved,
+ f8::numeric > 1::numeric AS f8numericpreserved
+FROM testvalues;
--
2.7.3
>From 4d721ac45967f64415bafb82039a652f0a451e2c Mon Sep 17 00:00:00 2001
From: Chapman Flack <c...@anastigmatix.net>
Date: Mon, 26 Feb 2018 01:26:01 -0500
Subject: [PATCH 2/2] Proof of concept fix of float[48]_numeric.
This patch simply uses the new FLT_DECIMAL_DIG/DBL_DECIMAL_DIG
constants in place of the FLT_DIG/DBL_DIG, just to see what happens.
It uses the double-underscore versions of the new constants, to
avoid mucking with the build procedure to change the C standard
to C11 where they would be visible without underscores. Needless to say,
this POC patch won't work where those constants aren't available, and
isn't proposed as a final patch.
It passes the newly added cast-no-precision-loss regression test.
It changes the output of an existing numeric test and a few numeric
aggregate tests.
I have adjusted the numeric test: its purpose is to check the direction
of numeric rounding of ties, but the value to be rounded was computed
in float8 and then cast to numeric, and because negative powers of ten
aren't tidy in binary, it can turn out that the float8 computation
produces a correctly-rounded-53-bit-result that is on the nearer-to-zero
side of a tie, and now that that result is correctly cast, the resulting
numeric doesn't round in the away-from-zero direction.
I changed that test because I concluded it wasn't meant to test
float8-to-numeric casting, but only the rounding of tied numerics,
so I just made the inner expression be typed numeric, and the expected
output is unchanged.
The three aggregate tests with changed output are working from a table
of float4 values, and my assumption is they are now simply producing
the correct aggregate results given the full precision of the input
values, but I haven't confirmed that yet, so this patch leaves those
three failing for now.
---
src/backend/utils/adt/numeric.c | 4 ++--
src/test/regress/expected/numeric.out | 12 ++++++------
src/test/regress/sql/numeric.sql | 12 ++++++------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 6f40072..c05de08 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -3224,7 +3224,7 @@ float8_numeric(PG_FUNCTION_ARGS)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot convert infinity to numeric")));
- snprintf(buf, sizeof(buf), "%.*g", DBL_DIG, val);
+ snprintf(buf, sizeof(buf), "%.*g", __DBL_DECIMAL_DIG__, val);
init_var(&result);
@@ -3295,7 +3295,7 @@ float4_numeric(PG_FUNCTION_ARGS)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot convert infinity to numeric")));
- snprintf(buf, sizeof(buf), "%.*g", FLT_DIG, val);
+ snprintf(buf, sizeof(buf), "%.*g", __FLT_DECIMAL_DIG__, val);
init_var(&result);
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index c186385..0c757a3 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -753,12 +753,12 @@ SELECT a, ceil(a), ceiling(a), floor(a), round(a) FROM ceil_floor_round;
DROP TABLE ceil_floor_round;
-- Check rounding, it should round ties away from zero.
SELECT i as pow,
- round((-2.5 * 10 ^ i)::numeric, -i),
- round((-1.5 * 10 ^ i)::numeric, -i),
- round((-0.5 * 10 ^ i)::numeric, -i),
- round((0.5 * 10 ^ i)::numeric, -i),
- round((1.5 * 10 ^ i)::numeric, -i),
- round((2.5 * 10 ^ i)::numeric, -i)
+ round((-2.5 * 10::numeric ^ i), -i),
+ round((-1.5 * 10::numeric ^ i), -i),
+ round((-0.5 * 10::numeric ^ i), -i),
+ round((0.5 * 10::numeric ^ i), -i),
+ round((1.5 * 10::numeric ^ i), -i),
+ round((2.5 * 10::numeric ^ i), -i)
FROM generate_series(-5,5) AS t(i);
pow | round | round | round | round | round | round
-----+----------+----------+----------+---------+---------+---------
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index dab9ae2..c096c43 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -677,12 +677,12 @@ DROP TABLE ceil_floor_round;
-- Check rounding, it should round ties away from zero.
SELECT i as pow,
- round((-2.5 * 10 ^ i)::numeric, -i),
- round((-1.5 * 10 ^ i)::numeric, -i),
- round((-0.5 * 10 ^ i)::numeric, -i),
- round((0.5 * 10 ^ i)::numeric, -i),
- round((1.5 * 10 ^ i)::numeric, -i),
- round((2.5 * 10 ^ i)::numeric, -i)
+ round((-2.5 * 10::numeric ^ i), -i),
+ round((-1.5 * 10::numeric ^ i), -i),
+ round((-0.5 * 10::numeric ^ i), -i),
+ round((0.5 * 10::numeric ^ i), -i),
+ round((1.5 * 10::numeric ^ i), -i),
+ round((2.5 * 10::numeric ^ i), -i)
FROM generate_series(-5,5) AS t(i);
-- Testing for width_bucket(). For convenience, we test both the
--
2.7.3
