From: Noah Misch <noah@leadboat.com>
To: Fujii Masao <masao.fujii@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: dblink query interruptibility
Date: Wed, 24 Jan 2024 12:45:32 -0800
Message-ID: <20240124204532.1b.nmisch@google.com> (raw)
In-Reply-To: <CAHGQGwGpDTXeg8K1oTmDv9nankaKTrCD-XW-tnkzo6=E9p5=uw@mail.gmail.com>
References: <20231122012945.74@rfd.leadboat.com>
<CAHGQGwGpDTXeg8K1oTmDv9nankaKTrCD-XW-tnkzo6=E9p5=uw@mail.gmail.com>
On Thu, Jan 25, 2024 at 04:23:39AM +0900, Fujii Masao wrote:
> I found that this patch was committed at d3c5f37dd5 and changed the
> error message in postgres_fdw slightly. Here's an example:
>
> #1. Begin a new transaction.
> #2. Execute a query accessing to a foreign table, like SELECT * FROM
> <foreign table>
> #3. Terminate the *remote* session corresponding to the foreign table.
> #4. Commit the transaction, and then currently the following error
> message is output.
>
> ERROR: FATAL: terminating connection due to administrator command
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> invalid socket
>
> Previously, before commit d3c5f37dd5, the error message at #4 did not
> include "invalid socket." Now, after the commit, it does. Is this
> change intentional?
No. It's a consequence of an intentional change in libpq call sequence, but I
was unaware I was changing an error message.
> + /* Consume whatever data is available from the socket */
> + if (PQconsumeInput(conn) == 0)
> + {
> + /* trouble; expect PQgetResult() to return NULL */
> + break;
> + }
> + }
> +
> + /* Now we can collect and return the next PGresult */
> + return PQgetResult(conn);
>
> This code appears to cause the change. When the remote session ends,
> PQconsumeInput() returns 0 and marks conn->socket as invalid.
> Subsequent PQgetResult() calls pqWait(), detecting the invalid socket
> and appending "invalid socket" to the error message.
>
> I think the "invalid socket" message is unsuitable in this scenario,
> and PQgetResult() should not be called after PQconsumeInput() returns
> 0. Thought?
The documentation is absolute about the necessity of PQgetResult():
PQsendQuery cannot be called again (on the same connection) until
PQgetResult has returned a null pointer, indicating that the command is
done.
PQgetResult must be called repeatedly until it returns a null pointer,
indicating that the command is done. (If called when no command is active,
PQgetResult will just return a null pointer at once.)
Similar statements also appear in libpq-pipeline-results,
libpq-pipeline-errors, and libpq-copy.
So, unless the documentation or my reading of it is wrong there, I think the
answer is something other than skipping PQgetResult(). Perhaps PQgetResult()
should not append "invalid socket" in this case? The extra line is a net
negative, though it's not wrong and not awful.
Thanks for reporting the change.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, masao.fujii@gmail.com
Subject: Re: dblink query interruptibility
In-Reply-To: <20240124204532.1b.nmisch@google.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox