Re: refactoring - share str2*int64 functions

From: Andres Freund <andres(at)anarazel(dot)de>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: refactoring - share str2*int64 functions
Date: 2019-07-19 04:16:22
Message-ID: 20190719041622.62wdeygdouvvu2xq@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-07-19 04:59:43 Re: Intermittent pg_ctl failures on Windows
Previous Message Amit Kapila 2019-07-19 04:10:02 Re: POC: Cleaning up orphaned files using undo logs