On Fri, Apr 2, 2010 at 17:26, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Thu, Mar 25, 2010 at 15:33, Fujii Masao <masao.fujii@gmail.com> wrote:
>>> Sorry for the delay. The attached patch replaces PQexec() used by dblink
>>> and libpqwalreceiver with pgwin32_PQexec() which is the win32 version of
>>> PQexec().
>>>
>>> pgwin32_PQexec() is provided as the library 'libpqbe.dll', which is created
>>> only on win32. dblink.dll and libpqwalreceiver.dll refer to libpqbe.dll.
>>> Also libpqbe.dll refers to libpq.dll.
>
>> [ assorted objections ]
>
> I disapprove of the whole approach, actually. The right way to fix this
> is to not touch or replace libpq at all, but to change walreceiver to
> use libpq's async-query facilities directly. Instead of PQexec, use
> PQsendQuery and then a loop involving PQisBusy, PQgetResult, etc.
> You've more or less done that loop, but you've put it in the wrong
> place.
Any particular reason not to wrap that in a function? Not called
pgwin32_PQexec() then, but something more generic? And not doing any
#defines to change PQexec, but call that wrapper directly?
> The larger point is that I don't believe this issue exists only on
> Windows. I think we're going to want something like this on all
> platforms, and that implies supporting poll() not just select() for the
> waiting part.
The most important part of the issue doesn't (because PQexec will be
interrupted by a signal), but there may certainly be others.
> The patch also seems confused about whether it's intending to be a
> general-purpose solution or not. You can maybe take some shortcuts
> if it's only going to be for walreceiver, but if you're going to put
> it in dblink it is *not* acceptable to take shortcuts like not
> processing errors completely.
Yeah, good point.
-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
