1 files changed, 116 insertions, 158 deletions

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index c459a842fa9..f98805fb5f7 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -101,8 +101,8 @@ static void materializeQueryResult(FunctionCallInfo fcinfo,
 								   const char *conname,
 								   const char *sql,
 								   bool fail);
-static PGresult *storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql);
-static void storeRow(volatile storeInfo *sinfo, PGresult *res, bool first);
+static PGresult *storeQueryResult(storeInfo *sinfo, PGconn *conn, const char *sql);
+static void storeRow(storeInfo *sinfo, PGresult *res, bool first);
 static remoteConn *getConnectionByName(const char *name);
 static HTAB *createConnHash(void);
 static remoteConn *createNewConnection(const char *name);
@@ -169,14 +169,6 @@ typedef struct remoteConnHashEnt
 /* initial number of connection hashes */
 #define NUMCONN 16
 
-static char *
-xpstrdup(const char *in)
-{
-	if (in == NULL)
-		return NULL;
-	return pstrdup(in);
-}
-
 pg_noreturn static void
 dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
 {
@@ -242,7 +234,7 @@ dblink_get_conn(char *conname_or_str,
 		}
 
 		PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
-							gettext_noop("received message via remote connection"));
+							"received message via remote connection");
 
 		dblink_security_check(conn, NULL, connstr);
 		if (PQclientEncoding(conn) != GetDatabaseEncoding())
@@ -343,7 +335,7 @@ dblink_connect(PG_FUNCTION_ARGS)
 	}
 
 	PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
-						gettext_noop("received message via remote connection"));
+						"received message via remote connection");
 
 	/* check password actually used if not superuser */
 	dblink_security_check(conn, connname, connstr);
@@ -870,131 +862,123 @@ static void
 materializeResult(FunctionCallInfo fcinfo, PGconn *conn, PGresult *res)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	TupleDesc	tupdesc;
+	bool		is_sql_cmd;
+	int			ntuples;
+	int			nfields;
 
 	/* prepTuplestoreResult must have been called previously */
 	Assert(rsinfo->returnMode == SFRM_Materialize);
 
-	PG_TRY();
+	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
-		TupleDesc	tupdesc;
-		bool		is_sql_cmd;
-		int			ntuples;
-		int			nfields;
+		is_sql_cmd = true;
 
-		if (PQresultStatus(res) == PGRES_COMMAND_OK)
-		{
-			is_sql_cmd = true;
+		/*
+		 * need a tuple descriptor representing one TEXT column to return the
+		 * command status string as our result tuple
+		 */
+		tupdesc = CreateTemplateTupleDesc(1);
+		TupleDescInitEntry(tupdesc, (AttrNumber) 1, "status",
+						   TEXTOID, -1, 0);
+		ntuples = 1;
+		nfields = 1;
+	}
+	else
+	{
+		Assert(PQresultStatus(res) == PGRES_TUPLES_OK);
 
-			/*
-			 * need a tuple descriptor representing one TEXT column to return
-			 * the command status string as our result tuple
-			 */
-			tupdesc = CreateTemplateTupleDesc(1);
-			TupleDescInitEntry(tupdesc, (AttrNumber) 1, "status",
-							   TEXTOID, -1, 0);
-			ntuples = 1;
-			nfields = 1;
-		}
-		else
-		{
-			Assert(PQresultStatus(res) == PGRES_TUPLES_OK);
+		is_sql_cmd = false;
 
-			is_sql_cmd = false;
+		/* get a tuple descriptor for our result type */
+		switch (get_call_result_type(fcinfo, NULL, &tupdesc))
+		{
+			case TYPEFUNC_COMPOSITE:
+				/* success */
+				break;
+			case TYPEFUNC_RECORD:
+				/* failed to determine actual type of RECORD */
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("function returning record called in context "
+								"that cannot accept type record")));
+				break;
+			default:
+				/* result type isn't composite */
+				elog(ERROR, "return type must be a row type");
+				break;
+		}
 
-			/* get a tuple descriptor for our result type */
-			switch (get_call_result_type(fcinfo, NULL, &tupdesc))
-			{
-				case TYPEFUNC_COMPOSITE:
-					/* success */
-					break;
-				case TYPEFUNC_RECORD:
-					/* failed to determine actual type of RECORD */
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("function returning record called in context "
-									"that cannot accept type record")));
-					break;
-				default:
-					/* result type isn't composite */
-					elog(ERROR, "return type must be a row type");
-					break;
-			}
+		/* make sure we have a persistent copy of the tupdesc */
+		tupdesc = CreateTupleDescCopy(tupdesc);
+		ntuples = PQntuples(res);
+		nfields = PQnfields(res);
+	}
 
-			/* make sure we have a persistent copy of the tupdesc */
-			tupdesc = CreateTupleDescCopy(tupdesc);
-			ntuples = PQntuples(res);
-			nfields = PQnfields(res);
-		}
+	/*
+	 * check result and tuple descriptor have the same number of columns
+	 */
+	if (nfields != tupdesc->natts)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATATYPE_MISMATCH),
+				 errmsg("remote query result rowtype does not match "
+						"the specified FROM clause rowtype")));
 
-		/*
-		 * check result and tuple descriptor have the same number of columns
-		 */
-		if (nfields != tupdesc->natts)
-			ereport(ERROR,
-					(errcode(ERRCODE_DATATYPE_MISMATCH),
-					 errmsg("remote query result rowtype does not match "
-							"the specified FROM clause rowtype")));
+	if (ntuples > 0)
+	{
+		AttInMetadata *attinmeta;
+		int			nestlevel = -1;
+		Tuplestorestate *tupstore;
+		MemoryContext oldcontext;
+		int			row;
+		char	  **values;
 
-		if (ntuples > 0)
-		{
-			AttInMetadata *attinmeta;
-			int			nestlevel = -1;
-			Tuplestorestate *tupstore;
-			MemoryContext oldcontext;
-			int			row;
-			char	  **values;
+		attinmeta = TupleDescGetAttInMetadata(tupdesc);
 
-			attinmeta = TupleDescGetAttInMetadata(tupdesc);
+		/* Set GUCs to ensure we read GUC-sensitive data types correctly */
+		if (!is_sql_cmd)
+			nestlevel = applyRemoteGucs(conn);
 
-			/* Set GUCs to ensure we read GUC-sensitive data types correctly */
-			if (!is_sql_cmd)
-				nestlevel = applyRemoteGucs(conn);
+		oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+		tupstore = tuplestore_begin_heap(true, false, work_mem);
+		rsinfo->setResult = tupstore;
+		rsinfo->setDesc = tupdesc;
+		MemoryContextSwitchTo(oldcontext);
 
-			oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
-			tupstore = tuplestore_begin_heap(true, false, work_mem);
-			rsinfo->setResult = tupstore;
-			rsinfo->setDesc = tupdesc;
-			MemoryContextSwitchTo(oldcontext);
+		values = palloc_array(char *, nfields);
 
-			values = palloc_array(char *, nfields);
+		/* put all tuples into the tuplestore */
+		for (row = 0; row < ntuples; row++)
+		{
+			HeapTuple	tuple;
 
-			/* put all tuples into the tuplestore */
-			for (row = 0; row < ntuples; row++)
+			if (!is_sql_cmd)
 			{
-				HeapTuple	tuple;
+				int			i;
 
-				if (!is_sql_cmd)
-				{
-					int			i;
-
-					for (i = 0; i < nfields; i++)
-					{
-						if (PQgetisnull(res, row, i))
-							values[i] = NULL;
-						else
-							values[i] = PQgetvalue(res, row, i);
-					}
-				}
-				else
+				for (i = 0; i < nfields; i++)
 				{
-					values[0] = PQcmdStatus(res);
+					if (PQgetisnull(res, row, i))
+						values[i] = NULL;
+					else
+						values[i] = PQgetvalue(res, row, i);
 				}
-
-				/* build the tuple and put it into the tuplestore. */
-				tuple = BuildTupleFromCStrings(attinmeta, values);
-				tuplestore_puttuple(tupstore, tuple);
+			}
+			else
+			{
+				values[0] = PQcmdStatus(res);
 			}
 
-			/* clean up GUC settings, if we changed any */
-			restoreLocalGucs(nestlevel);
+			/* build the tuple and put it into the tuplestore. */
+			tuple = BuildTupleFromCStrings(attinmeta, values);
+			tuplestore_puttuple(tupstore, tuple);
 		}
+
+		/* clean up GUC settings, if we changed any */
+		restoreLocalGucs(nestlevel);
 	}
-	PG_FINALLY();
-	{
-		/* be sure to release the libpq result */
-		PQclear(res);
-	}
-	PG_END_TRY();
+
+	PQclear(res);
 }
 
 /*
@@ -1013,16 +997,17 @@ materializeQueryResult(FunctionCallInfo fcinfo,
 					   bool fail)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
-	PGresult   *volatile res = NULL;
-	volatile storeInfo sinfo = {0};
 
 	/* prepTuplestoreResult must have been called previously */
 	Assert(rsinfo->returnMode == SFRM_Materialize);
 
-	sinfo.fcinfo = fcinfo;
-
+	/* Use a PG_TRY block to ensure we pump libpq dry of results */
 	PG_TRY();
 	{
+		storeInfo	sinfo = {0};
+		PGresult   *res;
+
+		sinfo.fcinfo = fcinfo;
 		/* Create short-lived memory context for data conversions */
 		sinfo.tmpcontext = AllocSetContextCreate(CurrentMemoryContext,
 												 "dblink temporary context",
@@ -1035,14 +1020,7 @@ materializeQueryResult(FunctionCallInfo fcinfo,
 			(PQresultStatus(res) != PGRES_COMMAND_OK &&
 			 PQresultStatus(res) != PGRES_TUPLES_OK))
 		{
-			/*
-			 * dblink_res_error will clear the passed PGresult, so we need
-			 * this ugly dance to avoid doing so twice during error exit
-			 */
-			PGresult   *res1 = res;
-
-			res = NULL;
-			dblink_res_error(conn, conname, res1, fail,
+			dblink_res_error(conn, conname, res, fail,
 							 "while executing query");
 			/* if fail isn't set, we'll return an empty query result */
 		}
@@ -1081,7 +1059,6 @@ materializeQueryResult(FunctionCallInfo fcinfo,
 			tuplestore_puttuple(tupstore, tuple);
 
 			PQclear(res);
-			res = NULL;
 		}
 		else
 		{
@@ -1090,26 +1067,20 @@ materializeQueryResult(FunctionCallInfo fcinfo,
 			Assert(rsinfo->setResult != NULL);
 
 			PQclear(res);
-			res = NULL;
 		}
 
 		/* clean up data conversion short-lived memory context */
 		if (sinfo.tmpcontext != NULL)
 			MemoryContextDelete(sinfo.tmpcontext);
-		sinfo.tmpcontext = NULL;
 
 		PQclear(sinfo.last_res);
-		sinfo.last_res = NULL;
 		PQclear(sinfo.cur_res);
-		sinfo.cur_res = NULL;
 	}
 	PG_CATCH();
 	{
-		/* be sure to release any libpq result we collected */
-		PQclear(res);
-		PQclear(sinfo.last_res);
-		PQclear(sinfo.cur_res);
-		/* and clear out any pending data in libpq */
+		PGresult   *res;
+
+		/* be sure to clear out any pending data in libpq */
 		while ((res = libpqsrv_get_result(conn, dblink_we_get_result)) !=
 			   NULL)
 			PQclear(res);
@@ -1122,7 +1093,7 @@ materializeQueryResult(FunctionCallInfo fcinfo,
  * Execute query, and send any result rows to sinfo->tuplestore.
  */
 static PGresult *
-storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql)
+storeQueryResult(storeInfo *sinfo, PGconn *conn, const char *sql)
 {
 	bool		first = true;
 	int			nestlevel = -1;
@@ -1190,7 +1161,7 @@ storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql)
  * (in this case the PGresult might contain either zero or one row).
  */
 static void
-storeRow(volatile storeInfo *sinfo, PGresult *res, bool first)
+storeRow(storeInfo *sinfo, PGresult *res, bool first)
 {
 	int			nfields = PQnfields(res);
 	HeapTuple	tuple;
@@ -2795,10 +2766,13 @@ dblink_connstr_check(const char *connstr)
 /*
  * Report an error received from the remote server
  *
- * res: the received error result (will be freed)
+ * res: the received error result
  * fail: true for ERROR ereport, false for NOTICE
  * fmt and following args: sprintf-style format and values for errcontext;
  * the resulting string should be worded like "while <some action>"
+ *
+ * If "res" is not NULL, it'll be PQclear'ed here (unless we throw error,
+ * in which case memory context cleanup will clear it eventually).
  */
 static void
 dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
@@ -2806,15 +2780,11 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 {
 	int			level;
 	char	   *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
-	char	   *pg_diag_message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
-	char	   *pg_diag_message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
-	char	   *pg_diag_message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
-	char	   *pg_diag_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
+	char	   *message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
+	char	   *message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
+	char	   *message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
+	char	   *message_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
 	int			sqlstate;
-	char	   *message_primary;
-	char	   *message_detail;
-	char	   *message_hint;
-	char	   *message_context;
 	va_list		ap;
 	char		dblink_context_msg[512];
 
@@ -2832,11 +2802,6 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 	else
 		sqlstate = ERRCODE_CONNECTION_FAILURE;
 
-	message_primary = xpstrdup(pg_diag_message_primary);
-	message_detail = xpstrdup(pg_diag_message_detail);
-	message_hint = xpstrdup(pg_diag_message_hint);
-	message_context = xpstrdup(pg_diag_context);
-
 	/*
 	 * If we don't get a message from the PGresult, try the PGconn.  This is
 	 * needed because for connection-level failures, PQgetResult may just
@@ -2846,14 +2811,6 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 		message_primary = pchomp(PQerrorMessage(conn));
 
 	/*
-	 * Now that we've copied all the data we need out of the PGresult, it's
-	 * safe to free it.  We must do this to avoid PGresult leakage.  We're
-	 * leaking all the strings too, but those are in palloc'd memory that will
-	 * get cleaned up eventually.
-	 */
-	PQclear(res);
-
-	/*
 	 * Format the basic errcontext string.  Below, we'll add on something
 	 * about the connection name.  That's a violation of the translatability
 	 * guidelines about constructing error messages out of parts, but since
@@ -2877,6 +2834,7 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 						 dblink_context_msg, conname)) :
 			 (errcontext("%s on unnamed dblink connection",
 						 dblink_context_msg))));
+	PQclear(res);
 }
 
 /*