pg.ddx.io  pgsql-www@postgresql.org mailing list archive  
help / color / mirror / Atom feed
NaNs in numeric_power (was Re: Postgres 11 release notes)
7+ messages / 4 participants
[nested] [flat]

Commits from this thread:
  2018-05-17 d1fc750b51  Make numeric power() handle NaNs according to the modern POSIX spec.
* NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-15 21:55  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 3 replies; 7+ messages in thread

From: Tom Lane @ 2018-05-15 21:55 UTC (permalink / raw)
  To: David Rowley <david.rowley@2ndquadrant.com>; +Cc: Bruce Momjian <bruce@momjian.us>; Đặng Minh Hướng <kakalot49@gmail.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

David Rowley <david.rowley@2ndquadrant.com> writes:
> On 16 May 2018 at 02:01, Tom Lane <tgl@sss.pgh.pa.us> 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

Attachments:

make-numeric_power-handle-NaNs-per-POSIX.patch (text/x-diff, 2.5K) download 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.0000000000000000 (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;
^ permalink raw reply [nested|flat] 7+ messages in thread

* Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-16 07:15  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  2 siblings, 1 reply; 7+ messages in thread

From: Dean Rasheed @ 2018-05-16 07:15 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: David Rowley <david.rowley@2ndquadrant.com>; Bruce Momjian <bruce@momjian.us>; Đặng Minh Hướng <kakalot49@gmail.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On 15 May 2018 at 22:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> On 16 May 2018 at 02:01, Tom Lane <tgl@sss.pgh.pa.us> 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




^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-16 13:44  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Dean Rasheed <dean.a.rasheed@gmail.com>
  0 siblings, 1 reply; 7+ messages in thread

From: Tom Lane @ 2018-05-16 13:44 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: David Rowley <david.rowley@2ndquadrant.com>; Bruce Momjian <bruce@momjian.us>; Đặng Minh Hướng <kakalot49@gmail.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On 15 May 2018 at 22:55, Tom Lane <tgl@sss.pgh.pa.us> 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




^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-16 21:49  David Rowley <david.rowley@2ndquadrant.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  2 siblings, 0 replies; 7+ messages in thread

From: David Rowley @ 2018-05-16 21:49 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Đặng Minh Hướng <kakalot49@gmail.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On 16 May 2018 at 09:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> On 16 May 2018 at 02:01, Tom Lane <tgl@sss.pgh.pa.us> 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




^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-16 22:30  Dean Rasheed <dean.a.rasheed@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 7+ messages in thread

From: Dean Rasheed @ 2018-05-16 22:30 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: David Rowley <david.rowley@2ndquadrant.com>; Bruce Momjian <bruce@momjian.us>; Đặng Minh Hướng <kakalot49@gmail.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On 16 May 2018 at 14:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> 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




^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* RE: NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-17 09:51  Huong Dangminh <huo-dangminh@ys.jp.nec.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  2 siblings, 1 reply; 7+ messages in thread

From: Huong Dangminh @ 2018-05-17 09:51 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; David Rowley <david.rowley@2ndquadrant.com>; +Cc: Bruce Momjian <bruce@momjian.us>; Akio Iwaasa <aki-iwaasa@vt.jp.nec.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Hi, 

> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> David Rowley <david.rowley@2ndquadrant.com> writes:
> > On 16 May 2018 at 02:01, Tom Lane <tgl@sss.pgh.pa.us> 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/


^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
@ 2018-05-17 15:22  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Huong Dangminh <huo-dangminh@ys.jp.nec.com>
  0 siblings, 0 replies; 7+ messages in thread

From: Tom Lane @ 2018-05-17 15:22 UTC (permalink / raw)
  To: Huong Dangminh <huo-dangminh@ys.jp.nec.com>; +Cc: David Rowley <david.rowley@2ndquadrant.com>; Bruce Momjian <bruce@momjian.us>; Akio Iwaasa <aki-iwaasa@vt.jp.nec.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Huong Dangminh <huo-dangminh@ys.jp.nec.com> 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




^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2018-05-17 15:22 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-05-15 21:55 NaNs in numeric_power (was Re: Postgres 11 release notes) Tom Lane <tgl@sss.pgh.pa.us>
2018-05-16 07:15 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-05-16 13:44   ` Tom Lane <tgl@sss.pgh.pa.us>
2018-05-16 22:30     ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-05-16 21:49 ` David Rowley <david.rowley@2ndquadrant.com>
2018-05-17 09:51 ` Huong Dangminh <huo-dangminh@ys.jp.nec.com>
2018-05-17 15:22   ` 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