From c46e1b06e68cf761bacbfe5b6d35ccf5fcbbb39d Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 12 Apr 2023 02:17:56 +0900
Subject: [PATCH v2 1/2] postgres_fdw: Fix bug causing unnecessary wait for
 cancel request reply.

In the event of a local transaction being aborted while a query is
running on a remote server in postgres_fdw, the extension sends
a cancel request to the remote server. However, if PQgetCancel()
returned NULL and no cancel request was issued, postgres_fdw
could still wait for the reply to the cancel request, causing unnecessary
wait time with a 30 second timeout.

To fix this issue, this commit ensures that postgres_fdw only waits for
a reply if a cancel request was actually issued. Additionally,
this commit improves PQgetCancel() to set error messages in certain
error cases, such as when out of memory happens and malloc() fails.
Moreover, this commit enhances postgres_fdw to report a warning
message when PQgetCancel() returns NULL, explaining the reason for
the NULL value.
---
 contrib/postgres_fdw/connection.c |  8 +++
 src/interfaces/libpq/fe-connect.c | 81 ++++++++++++++++++-------------
 2 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 75d93d6ead..c8bebe7b14 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -1384,6 +1384,14 @@ pgfdw_cancel_query_begin(PGconn *conn)
 		}
 		PQfreeCancel(cancel);
 	}
+	else
+	{
+		ereport(WARNING,
+				(errcode(ERRCODE_CONNECTION_FAILURE),
+				 errmsg("could not send cancel request: %s",
+						pchomp(PQerrorMessage(conn)))));
+		return false;
+	}
 
 	return true;
 }
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index fcd3d0d9a3..6558d098b5 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4709,11 +4709,17 @@ PQgetCancel(PGconn *conn)
 		return NULL;
 
 	if (conn->sock == PGINVALID_SOCKET)
+	{
+		libpq_append_conn_error(conn, "connection is not open");
 		return NULL;
+	}
 
 	cancel = malloc(sizeof(PGcancel));
 	if (cancel == NULL)
+	{
+		libpq_append_conn_error(conn, "out of memory");
 		return NULL;
+	}
 
 	memcpy(&cancel->raddr, &conn->raddr, sizeof(SockAddr));
 	cancel->be_pid = conn->be_pid;
@@ -4724,40 +4730,49 @@ PQgetCancel(PGconn *conn)
 	cancel->keepalives_idle = -1;
 	cancel->keepalives_interval = -1;
 	cancel->keepalives_count = -1;
-	if (conn->pgtcp_user_timeout != NULL)
-	{
-		if (!parse_int_param(conn->pgtcp_user_timeout,
-							 &cancel->pgtcp_user_timeout,
-							 conn, "tcp_user_timeout"))
-			goto fail;
-	}
-	if (conn->keepalives != NULL)
-	{
-		if (!parse_int_param(conn->keepalives,
-							 &cancel->keepalives,
-							 conn, "keepalives"))
-			goto fail;
-	}
-	if (conn->keepalives_idle != NULL)
-	{
-		if (!parse_int_param(conn->keepalives_idle,
-							 &cancel->keepalives_idle,
-							 conn, "keepalives_idle"))
-			goto fail;
-	}
-	if (conn->keepalives_interval != NULL)
-	{
-		if (!parse_int_param(conn->keepalives_interval,
-							 &cancel->keepalives_interval,
-							 conn, "keepalives_interval"))
-			goto fail;
-	}
-	if (conn->keepalives_count != NULL)
+
+	/*
+	 * Parse and interpret the TCP connection parameters, but only
+	 * if the connection is not made through a Unix-domain socket.
+	 * This is because Unix-domain sockets do not require these parameters.
+	 */
+	if (cancel->raddr.addr.ss_family != AF_UNIX)
 	{
-		if (!parse_int_param(conn->keepalives_count,
-							 &cancel->keepalives_count,
-							 conn, "keepalives_count"))
-			goto fail;
+		if (conn->pgtcp_user_timeout != NULL)
+		{
+			if (!parse_int_param(conn->pgtcp_user_timeout,
+								 &cancel->pgtcp_user_timeout,
+								 conn, "tcp_user_timeout"))
+				goto fail;
+		}
+		if (conn->keepalives != NULL)
+		{
+			if (!parse_int_param(conn->keepalives,
+								 &cancel->keepalives,
+								 conn, "keepalives"))
+				goto fail;
+		}
+		if (conn->keepalives_idle != NULL)
+		{
+			if (!parse_int_param(conn->keepalives_idle,
+								 &cancel->keepalives_idle,
+								 conn, "keepalives_idle"))
+				goto fail;
+		}
+		if (conn->keepalives_interval != NULL)
+		{
+			if (!parse_int_param(conn->keepalives_interval,
+								 &cancel->keepalives_interval,
+								 conn, "keepalives_interval"))
+				goto fail;
+		}
+		if (conn->keepalives_count != NULL)
+		{
+			if (!parse_int_param(conn->keepalives_count,
+								 &cancel->keepalives_count,
+								 conn, "keepalives_count"))
+				goto fail;
+		}
 	}
 
 	return cancel;
-- 
2.40.0