pg.ddx.io pgsql-www@postgresql.org mailing list archive
help / color / mirror / Atom feedNaNs 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