Re: dblink query interruptibility

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.