Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply

From: Jelte Fennema <postgres(at)jeltef(dot)nl>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: etsuro(dot)fujita(at)gmail(dot)com, masao(dot)fujii(at)oss(dot)nttdata(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply
Date: 2023-04-21 07:39:37
Message-ID: CAGECzQThFfOSf_J87pXO1oLY5=45A0deC=x0LsrpWMhO7GSs4w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2023-04-21 07:49:53 Re: Incremental sort for access method with ordered scan support (amcanorderbyop)
Previous Message Richard Guo 2023-04-21 07:34:42 Improve list manipulation in several places