On Tue, Jul 12, 2022 at 4:46 AM Robert Haas <robertmhaas@gmail.com> wrote:
> I don't think that 0001 is buying us a whole lot, really. I prefer the
> style where we have PG-specific functions that behave differently on
> different platforms to the one where we call something that looks like
> a native OS function call on all platforms but on some of them it is
> secretly invoking a replacement implementation in src/port. The
> problem with the latter is it looks like you're using something that's
> universally supported and works the same way everywhere, but you're
> really not. If it were up to me, we'd have more pg_whatever() that
> calls whatever() on non-Windows and something else on Windows, rather
> than going in the direction that this patch takes us.
Hmm, but that's not what we're doing in general. For example, on
Windows we're redirecting open() to a replacement function of our own,
we're not using "pg_open()" in our code. That's not an example based
on AC_REPLACE_FUNCS, but there are plenty of those too. Isn't this
quite well established?
AFAIK we generally only use pg_whatever() when there's a good reason,
such as an incompatibility, a complication or a different abstraction
that you want to highlight to a reader. The reason here was
temporary: we couldn't implement standard pread/pwrite perfectly on
ancient HP-UX, but we *can* implement it on Windows, so the reason is
gone.
These particular pg_ prefixes have only been in our tree for a few
years and I was hoping to boot them out again before they stick, like
"Size". I like using standard interfaces where possible for the very
basic stuff, to de-weird our stuff.
> I like all of the other patches. Reducing the number of configure
> tests that we need seems like a really good idea.
Thanks for looking. Yeah, we could also be a little more aggressive
about removing configure tests, in the cases where it's just Windows
vs !Windows. "HAVE_XXX" tests that are always true on POSIX systems
at the level we require would then be unnecessary.