Hello Andres,
>> If a function reports an error to log, it should keep on doing it, otherwise
>> there would be a regression.
>
> Err, huh. Especially if we change the signature, I fail to see how it's
> a regression if we change the behaviour.
ISTM that we do not understand one another, because I'm only trying to
state the obvious. Currently error messages on overflow or syntax error
are displayed. I just mean that such messages must keep on being emitted
somehow otherwise there would be a regression.
sql> SELECT INT8 '12345678901234567890';
ERROR: value "12345678901234567890" is out of range for type bigint
LINE 1: SELECT INT8 '12345678901234567890';
>> Sure. There is not exit though, just messages to stderr and return false.
>
> I think it's a seriously bad idea to have a function that returns
> depending in the error case depending on whether it's frontend or
> backend code. We shouldn't do stuff like that, it just leads to bugs.
Ok, you really want two functions and getting read of the errorOK boolean.
I got that. The scanint8 already exists with its ereport ERROR vs return
based on a boolean, so the point is to get read of this kind of signature.
>>> The boolean makes the calling code harder to understand, the function
>>> slower,
>>
>> Hmmm. So I understand that you would prefer 2 functions, one raw (fast) one
>> and the other with the other with the better error reporting facity, and the
>> user must chose the one they like. I'm fine with that as well.
>
> Well, the one with error reporting would use the former.
Yep, possibly two functions, no boolean.
Having a common function will not escape the fact that there is no
exception mechanism for front-end, and none seems desirable just for
parsing ints. So if you want NOT to have a same function with return
something vs raises an error, that would make three functions.
One basic int parsing in the fe/be common part.
typedef enum { STRTOINT_OK, STRTOINT_OVERFLOW, STRTOINT_SYNTAX_ERROR }
strtoint_error;
strtoint_error pg_strtoint64(const char * str, int64 * result);
Then for backend, probably in backend somewhere:
// raises an exception on errors
void pg_strtoint64_log(const char * str, int64 * result) {
switch (pg_strtoint64) {
case STRTOINT_OK: return;
case STRTOINT...: ereport(ERROR, ...);
}
}
And for frontend, only in frontend somewhere:
// print to stderr and return false on error
bool pg_strtoint64_err(const char * str, int64 * result);
I'm unhappy with the function names though, feel free to improve.
--
Fabien.