On Tue, Jul 12, 2022 at 4:46 AM Robert Haas <robertmh...@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.
