Hi,
On 2019-07-19 12:21:27 +0900, Michael Paquier wrote:
> On Thu, Jul 18, 2019 at 07:57:41AM +0000, Fabien COELHO wrote:
> > I'm unhappy with the function names though, feel free to improve.
>
> I would have something rather close to what you are suggesting, still
> not exactly that because we just don't care about the error strings
> generated for the frontend. And my bet is that each frontend would
> like to have their own error message depending on the error case.
Yea, the error messages pgbench is currently generating, for example,
don't make a lot of sense.
> FWIW, I had a similar experience with pg_strong_random() not so long
> ago, which required a backend-specific handling because the fallback
> random implementation needed some tweaks at postmaster startup (that's
> why we have an alias pg_backend_random in include/port.h). So I would
> recommend the following, roughly:
> - One set of functions in src/port/ which return the status code for
> the error handling, without any error reporting in it to avoid any
> ifdef FRONTEND business, which have a generic name pg_strto[u]intXX
> (XX = {16,32,64}). And have all that in a new, separate file, say
> strtoint.c?
Why not common? It's not a platform dependent bit. Could even be put
into the already existing string.c.
> - 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.
> - We also need the unsigned-specific equivalents of
> pg_mul_s64_overflow and such, so I would suggest putting that in a new
> header, simply uint.h. If I finish by committing this stuff, I would
> handle that in a separate commit.
Why not the same header? I fail to see what we'd gain by splitting it
up.
Greetings,
Andres Freund