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.

+    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. But that would in turn open an invitation for someone to implement it, and I'm not sure we want that. It could go either way, but we should be clear on what we want.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply via email to