Michael Paquier <michael.paquier@gmail.com> writes:
> On Sun, Jun 28, 2015 at 8:20 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, exactly. Unfortunately I see no way to add a useful test, at least
>> not one that will work in installcheck mode. There's no way to predict
>> what will be in the view in that case. Even for "make check", the output
>> would be pretty darn environment-dependent.
> And also because this patch had no review input regarding Windows and
> EXEC_BACKEND. I would suggest pinging the author (just did so),
> waiting for a fix a bit, and move on with 4. if nothing happens.
Well, mumble. After playing with this for a bit, I'm fairly convinced
that it offers useful functionality, especially with the error-reporting
additions I've proposed. Right now, there is no easy way to tell whether
a SIGHUP has worked, or why not if not, unless you have access to the
postmaster log. So I think there's definite usefulness here for
remote-administration scenarios.
So I kinda think that alternative 1 (document the Windows deficiency)
is better than having no such functionality at all. Obviously a proper
fix would be better yet, but that's something that could be rolled in
later.
> We usually require that a patch includes support for Windows as a
> requirement (see for example discussions about why pg_fincore in not a
> contrib module even if it overlaps a bit with pg_prewarm), why would
> this patch have a different treatment?
Agreed, but it was evidently not obvious to anyone that there was a
portability issue in this code, else we'd have resolved the issue
before it got committed. As a thought experiment, what would happen
if we'd not noticed this issue till post-release, which is certainly
not implausible?
Also, there are multiple pre-existing minor bugs (the leakage problem
I mentioned earlier, and some other things I've found while hacking
on the view patch) that we would have to deal with in some other
way if we revert now. I'd just as soon not detangle that.
regards, tom lane
