Re: refactoring - share str2*int64 functions - Mailing list pgsql-hackers

From Andres Freund
Subject Re: refactoring - share str2*int64 functions
Date
Msg-id 20190729050539.d7mbjabcrlv7bxc3@alap3.anarazel.de
Whole thread Raw
In response to Re: refactoring - share str2*int64 functions  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: refactoring - share str2*int64 functions
Next
From: Andres Freund
Date:
Subject: Re: LLVM compile failing in seawasp