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.