Re: refactoring - share str2*int64 functions

Hi,

On 2019-07-29 10:48:41 +0900, Michael Paquier wrote:
> On Thu, Jul 18, 2019 at 09:16:22PM -0700, Andres Freund wrote:
> >> - One set of functions for the backend, called pg_stro[u]intXX_backend
> >> or pg_backend_stro[u]intXX which can take as extra argument error_ok,
> >> calling the portions in src/port/, and move those functions in a new
> >> file prefixed with "backend_" in src/backend/utils/misc/ with a name
> >> consistent with the one in src/port/ (with the previous naming that
> >> would be backend_strtoint.c)
> >
> > I'm not following. What would be the point of any of this? The error_ok
> > bit is unnecessary, because the function is exactly the same as the
> > generic function. And the backend_ prefix would be pretty darn weird,
> > given that that's already below src/backend.
>
> Do you have a better idea of name for those functions?

I don't understand why they need any prefix. pg_strto[u]int{32,64}{,_checked}.
The unchecked variant would be for both frontend backend. The checked one
either for both frontend/backend, or just for backend. I also could live with
_raises, _throws or such instead of _checked. Implement all of them in one
file in /common/, possibly hidin gthe ones not currently implemented for the
frontend.

Imo if _checked is implemented for both frontend/backend they'd need
different error messages. In my opinion
out_of_range:
if (!errorOK)
fprintf(stderr,
"value \"%s\" is out of range for type bigint\n", str);
return false;

invalid_syntax:
if (!errorOK)
fprintf(stderr,
"invalid input syntax for type bigint: \"%s\"\n", str);

is unsuitable for generic code. In fact, I'm doubtful that it's applicable for
any use, except for int8in(), which makes me think it better ought to use the
a non-checked variant, and include the errors directly. If we still want to
have _checked - which is reasonable imo - it should refer to 64bit integers or somesuch.

Greetings,

Andres Freund