pg.ddx.io pgsql-hackers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
NaN divided by zero should yield NaN 3+ messages / 2 participants [nested] [flat]
Commits from this thread: 2020-07-20 4fb6aeb4f6 Make floating-point "NaN / 0" return NaN instead of raising an error.
* NaN divided by zero should yield NaN @ 2020-07-16 19:29 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 3+ messages in thread From: Tom Lane @ 2020-07-16 19:29 UTC (permalink / raw) To: pgsql-hackers@lists.postgresql.org; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Andres Freund <andres@anarazel.de> Dean Rasheed questioned this longstanding behavior: regression=# SELECT 'nan'::float8 / '0'::float8; ERROR: division by zero After a bit of research I think he's right: per IEEE 754 this should yield NaN, not an error. Accordingly I propose the attached patch. This is probably not something to back-patch, though. One thing that's not very clear to me is which of these spellings is preferable: if (unlikely(val2 == 0.0) && !isnan(val1)) if (unlikely(val2 == 0.0 && !isnan(val1))) I think we can reject this variant: if (unlikely(val2 == 0.0) && unlikely(!isnan(val1))) since actually the second condition *is* pretty likely. But I don't know which of the first two would give better code. Andres, any thoughts? regards, tom lane Attachments:^ permalink raw reply [nested|flat] 3+ messages in threadnan-over-zero-is-nan.patch (text/x-diff, 3.0K) download
diff --git a/src/include/utils/float.h b/src/include/utils/float.h index 8d4bbc51a6..e2aae8ef17 100644 --- a/src/include/utils/float.h +++ b/src/include/utils/float.h @@ -222,7 +222,7 @@ float4_div(const float4 val1, const float4 val2) { float4 result; - if (unlikely(val2 == 0.0f)) + if (unlikely(val2 == 0.0f) && !isnan(val1)) float_zero_divide_error(); result = val1 / val2; if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2)) @@ -238,7 +238,7 @@ float8_div(const float8 val1, const float8 val2) { float8 result; - if (unlikely(val2 == 0.0)) + if (unlikely(val2 == 0.0) && !isnan(val1)) float_zero_divide_error(); result = val1 / val2; if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2)) diff --git a/src/test/regress/expected/float4-misrounded-input.out b/src/test/regress/expected/float4-misrounded-input.out index 6c89af6394..38b847a1ad 100644 --- a/src/test/regress/expected/float4-misrounded-input.out +++ b/src/test/regress/expected/float4-misrounded-input.out @@ -143,6 +143,12 @@ SELECT 'nan'::float4 / 'nan'::float4; NaN (1 row) +SELECT 'nan'::float4 / '0'::float4; + ?column? +---------- + NaN +(1 row) + SELECT 'nan'::numeric::float4; float4 -------- diff --git a/src/test/regress/expected/float4.out b/src/test/regress/expected/float4.out index d6c22c1752..310402b6ee 100644 --- a/src/test/regress/expected/float4.out +++ b/src/test/regress/expected/float4.out @@ -143,6 +143,12 @@ SELECT 'nan'::float4 / 'nan'::float4; NaN (1 row) +SELECT 'nan'::float4 / '0'::float4; + ?column? +---------- + NaN +(1 row) + SELECT 'nan'::numeric::float4; float4 -------- diff --git a/src/test/regress/expected/float8.out b/src/test/regress/expected/float8.out index c635dd7dcb..2304b579d2 100644 --- a/src/test/regress/expected/float8.out +++ b/src/test/regress/expected/float8.out @@ -126,6 +126,12 @@ SELECT 'nan'::float8 / 'nan'::float8; NaN (1 row) +SELECT 'nan'::float8 / '0'::float8; + ?column? +---------- + NaN +(1 row) + SELECT 'nan'::numeric::float8; float8 -------- diff --git a/src/test/regress/sql/float4.sql b/src/test/regress/sql/float4.sql index 393d98fb14..1fcf823c21 100644 --- a/src/test/regress/sql/float4.sql +++ b/src/test/regress/sql/float4.sql @@ -50,6 +50,7 @@ SELECT ' INFINITY x'::float4; SELECT 'Infinity'::float4 + 100.0; SELECT 'Infinity'::float4 / 'Infinity'::float4; SELECT 'nan'::float4 / 'nan'::float4; +SELECT 'nan'::float4 / '0'::float4; SELECT 'nan'::numeric::float4; SELECT '' AS five, * FROM FLOAT4_TBL; diff --git a/src/test/regress/sql/float8.sql b/src/test/regress/sql/float8.sql index 288969aed6..f103871cdb 100644 --- a/src/test/regress/sql/float8.sql +++ b/src/test/regress/sql/float8.sql @@ -43,6 +43,7 @@ SELECT ' INFINITY x'::float8; SELECT 'Infinity'::float8 + 100.0; SELECT 'Infinity'::float8 / 'Infinity'::float8; SELECT 'nan'::float8 / 'nan'::float8; +SELECT 'nan'::float8 / '0'::float8; SELECT 'nan'::numeric::float8; SELECT '' AS five, * FROM FLOAT8_TBL;
* Re: NaN divided by zero should yield NaN @ 2020-07-17 18:08 Dean Rasheed <dean.a.rasheed@gmail.com> parent: Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 3+ messages in thread From: Dean Rasheed @ 2020-07-17 18:08 UTC (permalink / raw) To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Andres Freund <andres@anarazel.de> On Thu, 16 Jul 2020 at 20:29, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Dean Rasheed questioned this longstanding behavior: > > regression=# SELECT 'nan'::float8 / '0'::float8; > ERROR: division by zero > > After a bit of research I think he's right: per IEEE 754 this should > yield NaN, not an error. Accordingly I propose the attached patch. > This is probably not something to back-patch, though. > Agreed. > One thing that's not very clear to me is which of these spellings > is preferable: > > if (unlikely(val2 == 0.0) && !isnan(val1)) > if (unlikely(val2 == 0.0 && !isnan(val1))) > My guess is that the first would be better, since it would tell the compiler that it's unlikely to need to do the NaN test, so it would be kind of like doing if (unlikely(val2 == 0.0)) if (!isnan(val1))) Regards, Dean ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: NaN divided by zero should yield NaN @ 2020-07-20 23:46 Tom Lane <tgl@sss.pgh.pa.us> parent: Dean Rasheed <dean.a.rasheed@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2020-07-20 23:46 UTC (permalink / raw) To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Andres Freund <andres@anarazel.de> Dean Rasheed <dean.a.rasheed@gmail.com> writes: > On Thu, 16 Jul 2020 at 20:29, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> One thing that's not very clear to me is which of these spellings >> is preferable: >> if (unlikely(val2 == 0.0) && !isnan(val1)) >> if (unlikely(val2 == 0.0 && !isnan(val1))) > My guess is that the first would be better, since it would tell the > compiler that it's unlikely to need to do the NaN test, Yeah, that's the straightforward way to think about it, but I've found that gcc is sometimes less than straightforward ;-). Still, there's no obvious reason to do it the second way, so I pushed the first way. regards, tom lane ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2020-07-20 23:46 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-07-16 19:29 NaN divided by zero should yield NaN Tom Lane <tgl@sss.pgh.pa.us> 2020-07-17 18:08 ` Dean Rasheed <dean.a.rasheed@gmail.com> 2020-07-20 23:46 ` 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