On Tue, 16 Jun 2020 at 08:48, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote:
> On 2020-06-15 13:15, Ashutosh Bapat wrote: > > 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? > > I will add that. I wasn't actually sure about the precedence of these > operators, so 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 > > 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, then the message should also > reflect that. Yes. BTW, OUT_OF_RANGE is not exactly "undefined" either. I searched for an error code for "UNDEFINED" result but didn't find any. > But that would in turn open an invitation for someone to > implement it, and I'm not sure we want that. It will be more complex code, so difficult to implement but why do we prevent why not. > It could go either way, > but we should be clear on what we want. > > 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. -- Best Wishes, Ashutosh
