postgres_fdw: Useless test in pgfdw_exec_cleanup_query_end()

Started by Etsuro Fujitaalmost 2 years ago2 messages
#1Etsuro Fujita
etsuro.fujita@gmail.com
1 attachment(s)

Another thing I noticed while working on [1]/messages/by-id/CAPmGK15DF6EE7O6hTLbe5-fHvPDwEx9vm-BOCN3dsKOjZCo7bw@mail.gmail.com is $SUBJECT: this
function checks whether the given query string is non-NULL or not when
creating a WARNING message, but the function is always called with the
query string set, so it should be non-NULL. I removed the check and
instead added an assertion ensuring that the query string is non-NULL.
(I added the assertion to pgfdw_exec_cleanup_query_begin() as well.)
Attached is a patch for that.

If there are no objections, I will apply the patch to HEAD only.

Best regards,
Etsuro Fujita

[1]: /messages/by-id/CAPmGK15DF6EE7O6hTLbe5-fHvPDwEx9vm-BOCN3dsKOjZCo7bw@mail.gmail.com

Attachments:

remove-useless-test-in-pgfdw_exec_cleanup_query_end.patchapplication/octet-stream; name=remove-useless-test-in-pgfdw_exec_cleanup_query_end.patchDownload
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 4931ebf591..a766a2e4e3 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -1422,6 +1422,8 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
 static bool
 pgfdw_exec_cleanup_query_begin(PGconn *conn, const char *query)
 {
+	Assert(query != NULL);
+
 	/*
 	 * Submit a query.  Since we don't use non-blocking mode, this also can
 	 * block.  But its risk is relatively small, so we ignore that for now.
@@ -1443,6 +1445,8 @@ pgfdw_exec_cleanup_query_end(PGconn *conn, const char *query,
 	PGresult   *result = NULL;
 	bool		timed_out;
 
+	Assert(query != NULL);
+
 	/*
 	 * If requested, consume whatever data is available from the socket. (Note
 	 * that if all data is available, this allows pgfdw_get_cleanup_result to
@@ -1461,7 +1465,7 @@ pgfdw_exec_cleanup_query_end(PGconn *conn, const char *query,
 		if (timed_out)
 			ereport(WARNING,
 					(errmsg("could not get query result due to timeout"),
-					 query ? errcontext("remote SQL command: %s", query) : 0));
+					 errcontext("remote SQL command: %s", query)));
 		else
 			pgfdw_report_error(WARNING, NULL, conn, false, query);
 
#2Etsuro Fujita
etsuro.fujita@gmail.com
In reply to: Etsuro Fujita (#1)
Re: postgres_fdw: Useless test in pgfdw_exec_cleanup_query_end()

On Fri, Mar 22, 2024 at 9:30 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

If there are no objections, I will apply the patch to HEAD only.

Done.

Best regards,
Etsuro Fujita