On 12/21/2016 04:22 PM, Tom Lane wrote:
> Joe Conway <m...@joeconway.com> writes:
>> I did notice that postgres_fdw has the following stanza that I don't see
>> in dblink:
> 
>> 8<------------------
>> /*
>>  * If we don't get a message from the PGresult, try the PGconn.  This
>>  * is needed because for connection-level failures, PQexec may just
>>  * return NULL, not a PGresult at all.
>>  */
>> if (message_primary == NULL)
>>      message_primary = PQerrorMessage(conn);
>> 8<------------------
> 
>> I wonder if the original issue on pgsql-bugs was a connection-level
>> failure rather than OOM? Seems like dblink ought to do the same.
> 
> Oooh ... I had thought that code was in both, which was why I was having
> a hard time explaining the OP's failure.  But I see you're right,
> which provides a very straightforward explanation for the report.
> I believe that if libpq is unable to malloc a PGresult, it will return
> NULL but put an "out of memory" message into the PGconn's error buffer.
> I had supposed that we'd capture and report the latter, but as the
> dblink code stands, it won't.
> 
> In short, yes, please copy that bit into dblink.

The attached should do the trick I think. You think it is reasonable to
backpatch this part too?

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index ee45cd2..db3085a 100644
*** a/contrib/dblink/dblink.c
--- b/contrib/dblink/dblink.c
*************** static Relation get_rel_from_relname(tex
*** 112,118 ****
  static char *generate_relation_name(Relation rel);
  static void dblink_connstr_check(const char *connstr);
  static void dblink_security_check(PGconn *conn, remoteConn *rconn);
! static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
  static char *get_connect_string(const char *servername);
  static char *escape_param_str(const char *from);
  static void validate_pkattnums(Relation rel,
--- 112,119 ----
  static char *generate_relation_name(Relation rel);
  static void dblink_connstr_check(const char *connstr);
  static void dblink_security_check(PGconn *conn, remoteConn *rconn);
! static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
! 							 const char *dblink_context_msg, bool fail);
  static char *get_connect_string(const char *servername);
  static char *escape_param_str(const char *from);
  static void validate_pkattnums(Relation rel,
*************** dblink_open(PG_FUNCTION_ARGS)
*** 427,433 ****
  	res = PQexec(conn, buf.data);
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
! 		dblink_res_error(conname, res, "could not open cursor", fail);
  		PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
  	}
  
--- 428,434 ----
  	res = PQexec(conn, buf.data);
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
! 		dblink_res_error(conn, conname, res, "could not open cursor", fail);
  		PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
  	}
  
*************** dblink_close(PG_FUNCTION_ARGS)
*** 496,502 ****
  	res = PQexec(conn, buf.data);
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
! 		dblink_res_error(conname, res, "could not close cursor", fail);
  		PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
  	}
  
--- 497,503 ----
  	res = PQexec(conn, buf.data);
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
! 		dblink_res_error(conn, conname, res, "could not close cursor", fail);
  		PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
  	}
  
*************** dblink_fetch(PG_FUNCTION_ARGS)
*** 599,605 ****
  		(PQresultStatus(res) != PGRES_COMMAND_OK &&
  		 PQresultStatus(res) != PGRES_TUPLES_OK))
  	{
! 		dblink_res_error(conname, res, "could not fetch from cursor", fail);
  		return (Datum) 0;
  	}
  	else if (PQresultStatus(res) == PGRES_COMMAND_OK)
--- 600,607 ----
  		(PQresultStatus(res) != PGRES_COMMAND_OK &&
  		 PQresultStatus(res) != PGRES_TUPLES_OK))
  	{
! 		dblink_res_error(conn, conname, res,
! 						 "could not fetch from cursor", fail);
  		return (Datum) 0;
  	}
  	else if (PQresultStatus(res) == PGRES_COMMAND_OK)
*************** dblink_record_internal(FunctionCallInfo
*** 750,757 ****
  				if (PQresultStatus(res) != PGRES_COMMAND_OK &&
  					PQresultStatus(res) != PGRES_TUPLES_OK)
  				{
! 					dblink_res_error(conname, res, "could not execute query",
! 									 fail);
  					/* if fail isn't set, we'll return an empty query result */
  				}
  				else
--- 752,759 ----
  				if (PQresultStatus(res) != PGRES_COMMAND_OK &&
  					PQresultStatus(res) != PGRES_TUPLES_OK)
  				{
! 					dblink_res_error(conn, conname, res,
! 									 "could not execute query", fail);
  					/* if fail isn't set, we'll return an empty query result */
  				}
  				else
*************** materializeQueryResult(FunctionCallInfo
*** 996,1002 ****
  			PGresult   *res1 = res;
  
  			res = NULL;
! 			dblink_res_error(conname, res1, "could not execute query", fail);
  			/* if fail isn't set, we'll return an empty query result */
  		}
  		else if (PQresultStatus(res) == PGRES_COMMAND_OK)
--- 998,1005 ----
  			PGresult   *res1 = res;
  
  			res = NULL;
! 			dblink_res_error(conn, conname, res1,
! 							 "could not execute query", fail);
  			/* if fail isn't set, we'll return an empty query result */
  		}
  		else if (PQresultStatus(res) == PGRES_COMMAND_OK)
*************** dblink_exec(PG_FUNCTION_ARGS)
*** 1431,1437 ****
  			(PQresultStatus(res) != PGRES_COMMAND_OK &&
  			 PQresultStatus(res) != PGRES_TUPLES_OK))
  		{
! 			dblink_res_error(conname, res, "could not execute command", fail);
  
  			/*
  			 * and save a copy of the command status string to return as our
--- 1434,1441 ----
  			(PQresultStatus(res) != PGRES_COMMAND_OK &&
  			 PQresultStatus(res) != PGRES_TUPLES_OK))
  		{
! 			dblink_res_error(conn, conname, res,
! 							 "could not execute command", fail);
  
  			/*
  			 * and save a copy of the command status string to return as our
*************** dblink_connstr_check(const char *connstr
*** 2662,2668 ****
  }
  
  static void
! dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail)
  {
  	int			level;
  	char	   *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
--- 2666,2673 ----
  }
  
  static void
! dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
! 				 const char *dblink_context_msg, bool fail)
  {
  	int			level;
  	char	   *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
*************** dblink_res_error(const char *conname, PG
*** 2696,2701 ****
--- 2701,2714 ----
  	xpstrdup(message_hint, pg_diag_message_hint);
  	xpstrdup(message_context, pg_diag_context);
  
+ 	/*
+ 	 * If we don't get a message from the PGresult, try the PGconn.  This
+ 	 * is needed because for connection-level failures, PQexec may just
+ 	 * return NULL, not a PGresult at all.
+ 	 */
+ 	if (message_primary == NULL)
+ 		message_primary = PQerrorMessage(conn);
+ 
  	if (res)
  		PQclear(res);
  

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to