From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: factorial of negative numbers |
Date: | 2020-06-15 11:15:25 |
Message-ID: | CAExHW5uoXsBPXU4TWS5WE4S_AFyPkt=_3gf61_9-EUE67ALJNw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jun 15, 2020 at 12:41 PM Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>
> Adjacent to the discussion in [0] I wanted to document the factorial()
> function and expand the tests for that slightly with some edge cases.
>
> I noticed that the current implementation returns 1 for the factorial of
> all negative numbers:
>
> SELECT factorial(-4);
> factorial
> -----------
> 1
>
> While there are some advanced mathematical constructions that define
> factorials for negative numbers, they certainly produce different
> answers than this.
>
> Curiously, before the reimplementation of factorial using numeric
> (04a4821adef38155b7920ba9eb83c4c3c29156f8), it returned 0 for negative
> numbers, which is also not correct by any theory I could find.
>
> I propose to change this to error out for negative numbers.
+1.
Here are some comments
I see below in the .out but there's not corresponding SQL in .sql.
+SELECT factorial(-4);
+ factorial
+-----------
+ 1
+(1 row)
+
Should we also add -4! to cover both function as well as the operator?
+ if (num < 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
This looks more of ERRCODE_FEATURE_NOT_SUPPORTED esp. since factorial
of negative numbers is defined but we are not supporting it. I looked
at some other usages of this error code. All of them are really are
out of range value errors.
Otherwise the patches look good to me.
From | Date | Subject | |
---|---|---|---|
Next Message | Inoue, Hiroshi | 2020-06-15 11:50:23 | Re: Removal of currtid()/currtid2() and some table AM cleanup |
Previous Message | Bharath Rupireddy | 2020-06-15 11:09:04 | Re: Parallel copy |