Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
Huong Dangminh writes: > Thank you. The patch looks fine to me. > Also, I have done the "make check" in Windows and Linux environment with no > problem. Pushed, thanks for reviewing/testing. regards, tom lane
RE: NaNs in numeric_power (was Re: Postgres 11 release notes)
Hi, > From: Tom Lane [mailto:t...@sss.pgh.pa.us] > David Rowley writes: > > On 16 May 2018 at 02:01, Tom Lane wrote: > >> I'm not particularly fussed about getting credit for that. However, > >> looking again at how that patch series turned out --- ie, that we > >> ensured POSIX behavior for NaNs only in HEAD --- I wonder whether we > >> shouldn't do what was mentioned in the commit log for 6bdf1303, and > >> teach numeric_pow() about these same special cases. > >> It seems like it would be more consistent to change both functions > >> for v11, rather than letting that other shoe drop in some future > >> major release. > > > I'm inclined to agree. It's hard to imagine these two functions > > behaving differently in regards to NaN input is useful to anyone. > Thank you. The patch looks fine to me. Also, I have done the "make check" in Windows and Linux environment with no problem. Thanks and best regards, --- Dang Minh Huong NEC Solution Innovators, Ltd. http://www.nec-solutioninnovators.co.jp/en/
Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
On 16 May 2018 at 14:44, Tom Lane wrote: > Dean Rasheed writes: >> In the case 1 ^ NaN = 1, what should the result scale be? > > The result is exact, so I don't see a reason to be worried about its > scale. Messing with the scale would also require expending even > more code on what is, in the end, a corner case that no users have > expressed concern about. > OK, fine. Not really worth worrying about. Regards, Dean
Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
On 16 May 2018 at 09:55, Tom Lane wrote: > David Rowley writes: >> On 16 May 2018 at 02:01, Tom Lane wrote: >>> I'm not particularly fussed about getting credit for that. However, >>> looking again at how that patch series turned out --- ie, that >>> we ensured POSIX behavior for NaNs only in HEAD --- I wonder >>> whether we shouldn't do what was mentioned in the commit log for >>> 6bdf1303, and teach numeric_pow() about these same special cases. >>> It seems like it would be more consistent to change both functions >>> for v11, rather than letting that other shoe drop in some future >>> major release. > >> I'm inclined to agree. It's hard to imagine these two functions >> behaving differently in regards to NaN input is useful to anyone. > > Here's a proposed patch for that. Looks good to me. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
Dean Rasheed writes: > On 15 May 2018 at 22:55, Tom Lane wrote: >> Here's a proposed patch for that. > In the case 1 ^ NaN = 1, what should the result scale be? The result is exact, so I don't see a reason to be worried about its scale. Messing with the scale would also require expending even more code on what is, in the end, a corner case that no users have expressed concern about. regards, tom lane
Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
On 15 May 2018 at 22:55, Tom Lane wrote: > David Rowley writes: >> On 16 May 2018 at 02:01, Tom Lane wrote: >>> I'm not particularly fussed about getting credit for that. However, >>> looking again at how that patch series turned out --- ie, that >>> we ensured POSIX behavior for NaNs only in HEAD --- I wonder >>> whether we shouldn't do what was mentioned in the commit log for >>> 6bdf1303, and teach numeric_pow() about these same special cases. >>> It seems like it would be more consistent to change both functions >>> for v11, rather than letting that other shoe drop in some future >>> major release. > >> I'm inclined to agree. It's hard to imagine these two functions >> behaving differently in regards to NaN input is useful to anyone. > > Here's a proposed patch for that. > In the case 1 ^ NaN = 1, what should the result scale be? For other inputs, the result scale is at least as large as the scale of the first input, so I would suggest that the same ought to be the case here. Otherwise, this looks fine, and I agree that it makes thinks neater / more consistent. Regards, Dean
NaNs in numeric_power (was Re: Postgres 11 release notes)
David Rowley writes: > On 16 May 2018 at 02:01, Tom Lane wrote: >> I'm not particularly fussed about getting credit for that. However, >> looking again at how that patch series turned out --- ie, that >> we ensured POSIX behavior for NaNs only in HEAD --- I wonder >> whether we shouldn't do what was mentioned in the commit log for >> 6bdf1303, and teach numeric_pow() about these same special cases. >> It seems like it would be more consistent to change both functions >> for v11, rather than letting that other shoe drop in some future >> major release. > I'm inclined to agree. It's hard to imagine these two functions > behaving differently in regards to NaN input is useful to anyone. Here's a proposed patch for that. regards, tom lane diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index dcf31e3..8dfdffc 100644 *** a/src/backend/utils/adt/numeric.c --- b/src/backend/utils/adt/numeric.c *** numeric_power(PG_FUNCTION_ARGS) *** 2972,2981 NumericVar result; /* ! * Handle NaN */ ! if (NUMERIC_IS_NAN(num1) || NUMERIC_IS_NAN(num2)) PG_RETURN_NUMERIC(make_result(&const_nan)); /* * Initialize things --- 2972,2998 NumericVar result; /* ! * Handle NaN cases. We follow the POSIX spec for pow(3), which says that ! * NaN ^ 0 = 1, and 1 ^ NaN = 1, while all other cases with NaN inputs ! * yield NaN (with no error). */ ! if (NUMERIC_IS_NAN(num1)) ! { ! if (!NUMERIC_IS_NAN(num2)) ! { ! init_var_from_num(num2, &arg2); ! if (cmp_var(&arg2, &const_zero) == 0) ! PG_RETURN_NUMERIC(make_result(&const_one)); ! } ! PG_RETURN_NUMERIC(make_result(&const_nan)); ! } ! if (NUMERIC_IS_NAN(num2)) ! { ! init_var_from_num(num1, &arg1); ! if (cmp_var(&arg1, &const_one) == 0) ! PG_RETURN_NUMERIC(make_result(&const_one)); PG_RETURN_NUMERIC(make_result(&const_nan)); + } /* * Initialize things diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out index 17985e8..1cb3c3b 100644 *** a/src/test/regress/expected/numeric.out --- b/src/test/regress/expected/numeric.out *** select 0.0 ^ 12.34; *** 1664,1669 --- 1664,1700 0. (1 row) + -- NaNs + select 'NaN'::numeric ^ 'NaN'::numeric; + ?column? + -- + NaN + (1 row) + + select 'NaN'::numeric ^ 0; + ?column? + -- + 1 + (1 row) + + select 'NaN'::numeric ^ 1; + ?column? + -- + NaN + (1 row) + + select 0 ^ 'NaN'::numeric; + ?column? + -- + NaN + (1 row) + + select 1 ^ 'NaN'::numeric; + ?column? + -- + 1 + (1 row) + -- invalid inputs select 0.0 ^ (-12.34); ERROR: zero raised to a negative power is undefined diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql index d77504e..a939412 100644 *** a/src/test/regress/sql/numeric.sql --- b/src/test/regress/sql/numeric.sql *** select (-12.34) ^ 0.0; *** 911,916 --- 911,923 select 12.34 ^ 0.0; select 0.0 ^ 12.34; + -- NaNs + select 'NaN'::numeric ^ 'NaN'::numeric; + select 'NaN'::numeric ^ 0; + select 'NaN'::numeric ^ 1; + select 0 ^ 'NaN'::numeric; + select 1 ^ 'NaN'::numeric; + -- invalid inputs select 0.0 ^ (-12.34); select (-12.34) ^ 1.2;