Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply - Mailing list pgsql-hackers

From Jelte Fennema
Subject Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply
Date
Msg-id CAGECzQThFfOSf_J87pXO1oLY5=45A0deC=x0LsrpWMhO7GSs4w@mail.gmail.com
Whole thread Raw
In response to Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Responses Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply  (Fujii Masao <masao.fujii@oss.nttdata.com>)
List pgsql-hackers
> In my opinion, PQconnectPoll and PQgetCancel should use the same
> parsing function or PQconnectPoll should set parsed values, making
> unnecessary for PQgetCancel to parse the same parameter
> again.

Yes, I totally agree. So I think patch 0002 looks fine.

> Additionally, PQgetCancel should set appropriate error messages
> for all failure modes.

I don't think that PQgetCancel should ever set error messages on the
provided conn object though. It's not part of the documented API and
it's quite confusing since there's actually no error on the connection
itself. That this happens for the keepalive parameter was an
unintended sideeffect of 5987feb70b combined with the fact that the
parsing is different. All those parsing functions should never error,
because setting up the connection should already have checked them.

So I think the newly added libpq_append_conn_error calls in patch 0001
should be removed. The AF_UNIX check and the new WARNING in pg_fdw
seem fine though. It would probably make sense to have them be
separate patches though, because they are pretty unrelated.



pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Improve list manipulation in several places
Next
From: Richard Guo
Date:
Subject: Re: Incremental sort for access method with ordered scan support (amcanorderbyop)