Re: factorial of negative numbers

On Thu, Jun 18, 2020 at 1:57 PM Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 2020-06-18 09:43, Juan José Santamaría Flecha wrote: > > The gamma function from math.h returns a NaN for negative integer > > values, the postgres factorial function returns a numeric, which allows >

Re: factorial of negative numbers

On 2020-06-18 09:43, Juan José Santamaría Flecha wrote: On Thu, Jun 18, 2020 at 9:13 AM Peter Eisentraut > wrote: On 2020-06-16 14:17, Dean Rasheed wrote: > I think you're probably right though. Raising an out-of-range error > seems like

Re: factorial of negative numbers

On Thu, Jun 18, 2020 at 9:13 AM Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 2020-06-16 14:17, Dean Rasheed wrote: > > I think you're probably right though. Raising an out-of-range error > > seems like the best option. > > committed as proposed then > The gamma function from m

Re: factorial of negative numbers

On 2020-06-16 14:17, Dean Rasheed wrote: I think you're probably right though. Raising an out-of-range error seems like the best option. committed as proposed then -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: factorial of negative numbers

On Tue, 16 Jun 2020 at 12:18, Peter Eisentraut wrote: > > On 2020-06-16 11:49, Dean Rasheed wrote: > > With [1], we could return 'Infinity', which would be more correct from > > a mathematical point of view, and might be preferable to erroring-out > > in some contexts. > > But the limit of the gam

Re: factorial of negative numbers

On 2020-06-16 11:49, Dean Rasheed wrote: With [1], we could return 'Infinity', which would be more correct from a mathematical point of view, and might be preferable to erroring-out in some contexts. But the limit of the gamma function is either negative or positive infinity, depending on from

Re: factorial of negative numbers

On Tue, Jun 16, 2020 at 11:50 AM Dean Rasheed wrote: > On Tue, 16 Jun 2020 at 10:09, Juan José Santamaría Flecha > wrote: > > > > It is defined as NaN (or undefined), which is not in the realm of > integer numbers. You might get a clear idea of the logic from [1], where > they also make a case f

Re: factorial of negative numbers

On Tue, 16 Jun 2020 at 10:09, Juan José Santamaría Flecha wrote: > > It is defined as NaN (or undefined), which is not in the realm of integer > numbers. You might get a clear idea of the logic from [1], where they also > make a case for the error being ERRCODE_DIVISION_BY_ZERO. > > [1] http://m

Re: factorial of negative numbers

On Tue, 16 Jun 2020 at 09:55, Bruce Momjian wrote: > > On Tue, Jun 16, 2020 at 08:31:21AM +0100, Dean Rasheed wrote: > > > > Most common implementations do regard factorial as undefined for > > anything other than positive integers, as well as following the > > convention that factorial(0) = 1. So

Re: factorial of negative numbers

On Tue, Jun 16, 2020 at 10:55 AM Bruce Momjian wrote: > On Tue, Jun 16, 2020 at 08:31:21AM +0100, Dean Rasheed wrote: > > > > Most common implementations do regard factorial as undefined for > > anything other than positive integers, as well as following the > > convention that factorial(0) = 1.

Re: factorial of negative numbers

On Tue, Jun 16, 2020 at 08:31:21AM +0100, Dean Rasheed wrote: > On Tue, 16 Jun 2020 at 06:00, Ashutosh Bapat > wrote: > > > > Divison by zero is really undefined, 12345678 * 12345678 (just some > > numbers) is out of range of say int4, but factorial of a negative number > > has some meaning and

Re: factorial of negative numbers

On Tue, 16 Jun 2020 at 06:00, Ashutosh Bapat wrote: > > Divison by zero is really undefined, 12345678 * 12345678 (just some numbers) > is out of range of say int4, but factorial of a negative number has some > meaning and is defined but PostgreSQL does not support it. > Actually, I think undefi

Re: factorial of negative numbers

it is interesting as a regression test for that as well. > > +1. > > +if (num < 0) > > +ereport(ERROR, > > + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), > > > > This looks more of ERRCODE_FEATURE_NOT_SUPPORTED esp. since factorial > &g

Re: factorial of negative numbers

ED 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. The proposed error message says this is undefined. If we use an error code that says it's not implemented

Re: factorial of negative numbers

... oh, one slightly more important nit-pick: per the catalogs and code, the function is factorial(bigint): Schema | Name| Result data type | Argument data types | Type +---+--+-+-- pg_catalog | factorial | numeric |

Re: factorial of negative numbers

Peter Eisentraut writes: > 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 propose to change this to error out for negative numbers. +1 for all of this, with a couple trivial nitpicks about t

Re: factorial of negative numbers

(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.

factorial of negative numbers

00!; +SELECT 0!; +SELECT factorial(-4); -- 2.27.0 From 9ae0d1135d0a75b4560f2a4c22be8b8bbfde0803 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 15 Jun 2020 09:02:14 +0200 Subject: [PATCH 3/3] Disallow factorial of negative numbers The previous implementation returned 1 for all