Re: [BUGS] BUG #4203: perform dblink() in begin/exception returns wrong SQLSTATE code

Joe Conway <[email protected]>
Newsgroups gmane.comp.db.postgresql.devel.patches
Message-ID <[email protected]>
Tom Lane wrote:
> Joe Conway <[email protected]> writes:
>> [ improved patch ]
> 
> Still a couple quibbles:

[ more good feedback ]

All valid complaints, and noticeably improved/simplified code as a 
result. Third patch attached.

Joe


-- 
Sent via pgsql-patches mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
dblink.2008.06.01.3.diff (text/x-patch, 14.7 KB)
Index: dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.73
diff -c -r1.73 dblink.c
*** dblink.c	4 Apr 2008 17:02:56 -0000	1.73
--- dblink.c	1 Jun 2008 23:52:39 -0000
***************
*** 94,99 ****
--- 94,100 ----
  static Oid	get_relid_from_relname(text *relname_text);
  static char *generate_relation_name(Oid relid);
  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);
  
  /* Global */
  static remoteConn *pconn = NULL;
***************
*** 125,158 ****
  		} \
  	} while (0)
  
! #define DBLINK_RES_INTERNALERROR(p2) \
! 	do { \
! 			msg = pstrdup(PQerrorMessage(conn)); \
! 			if (res) \
! 				PQclear(res); \
! 			elog(ERROR, "%s: %s", p2, msg); \
! 	} while (0)
! 
! #define DBLINK_RES_ERROR(p2) \
  	do { \
! 			msg = pstrdup(PQerrorMessage(conn)); \
! 			if (res) \
! 				PQclear(res); \
! 			ereport(ERROR, \
! 					(errcode(ERRCODE_SYNTAX_ERROR), \
! 					 errmsg("%s", p2), \
! 					 errdetail("%s", msg))); \
  	} while (0)
  
! #define DBLINK_RES_ERROR_AS_NOTICE(p2) \
  	do { \
  			msg = pstrdup(PQerrorMessage(conn)); \
  			if (res) \
  				PQclear(res); \
! 			ereport(NOTICE, \
! 					(errcode(ERRCODE_SYNTAX_ERROR), \
! 					 errmsg("%s", p2), \
! 					 errdetail("%s", msg))); \
  	} while (0)
  
  #define DBLINK_CONN_NOT_AVAIL \
--- 126,145 ----
  		} \
  	} while (0)
  
! #define xpstrdup(var_c, var_) \
  	do { \
! 		if (var_ != NULL) \
! 			var_c = pstrdup(var_); \
! 		else \
! 			var_c = NULL; \
  	} while (0)
  
! #define DBLINK_RES_INTERNALERROR(p2) \
  	do { \
  			msg = pstrdup(PQerrorMessage(conn)); \
  			if (res) \
  				PQclear(res); \
! 			elog(ERROR, "%s: %s", p2, msg); \
  	} while (0)
  
  #define DBLINK_CONN_NOT_AVAIL \
***************
*** 396,408 ****
  	res = PQexec(conn, buf.data);
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
! 		if (fail)
! 			DBLINK_RES_ERROR("sql error");
! 		else
! 		{
! 			DBLINK_RES_ERROR_AS_NOTICE("sql error");
! 			PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
! 		}
  	}
  
  	PQclear(res);
--- 383,390 ----
  	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"));
  	}
  
  	PQclear(res);
***************
*** 470,482 ****
  	res = PQexec(conn, buf.data);
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
! 		if (fail)
! 			DBLINK_RES_ERROR("sql error");
! 		else
! 		{
! 			DBLINK_RES_ERROR_AS_NOTICE("sql error");
! 			PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
! 		}
  	}
  
  	PQclear(res);
--- 452,459 ----
  	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"));
  	}
  
  	PQclear(res);
***************
*** 513,519 ****
  	int			call_cntr;
  	int			max_calls;
  	AttInMetadata *attinmeta;
- 	char	   *msg;
  	PGresult   *res = NULL;
  	MemoryContext oldcontext;
  	char	   *conname = NULL;
--- 490,495 ----
***************
*** 590,602 ****
  			(PQresultStatus(res) != PGRES_COMMAND_OK &&
  			 PQresultStatus(res) != PGRES_TUPLES_OK))
  		{
! 			if (fail)
! 				DBLINK_RES_ERROR("sql error");
! 			else
! 			{
! 				DBLINK_RES_ERROR_AS_NOTICE("sql error");
! 				SRF_RETURN_DONE(funcctx);
! 			}
  		}
  		else if (PQresultStatus(res) == PGRES_COMMAND_OK)
  		{
--- 566,573 ----
  			(PQresultStatus(res) != PGRES_COMMAND_OK &&
  			 PQresultStatus(res) != PGRES_TUPLES_OK))
  		{
! 			dblink_res_error(conname, res, "could not fetch from cursor", fail);
! 			SRF_RETURN_DONE(funcctx);
  		}
  		else if (PQresultStatus(res) == PGRES_COMMAND_OK)
  		{
***************
*** 846,860 ****
  				(PQresultStatus(res) != PGRES_COMMAND_OK &&
  				 PQresultStatus(res) != PGRES_TUPLES_OK))
  			{
! 				if (fail)
! 					DBLINK_RES_ERROR("sql error");
! 				else
! 				{
! 					DBLINK_RES_ERROR_AS_NOTICE("sql error");
! 					if (freeconn)
! 						PQfinish(conn);
! 					SRF_RETURN_DONE(funcctx);
! 				}
  			}
  
  			if (PQresultStatus(res) == PGRES_COMMAND_OK)
--- 817,826 ----
  				(PQresultStatus(res) != PGRES_COMMAND_OK &&
  				 PQresultStatus(res) != PGRES_TUPLES_OK))
  			{
! 				dblink_res_error(conname, res, "could not execute query", fail);
! 				if (freeconn)
! 					PQfinish(conn);
! 				SRF_RETURN_DONE(funcctx);
  			}
  
  			if (PQresultStatus(res) == PGRES_COMMAND_OK)
***************
*** 1180,1189 ****
  		(PQresultStatus(res) != PGRES_COMMAND_OK &&
  		 PQresultStatus(res) != PGRES_TUPLES_OK))
  	{
! 		if (fail)
! 			DBLINK_RES_ERROR("sql error");
! 		else
! 			DBLINK_RES_ERROR_AS_NOTICE("sql error");
  
  		/* need a tuple descriptor representing one TEXT column */
  		tupdesc = CreateTemplateTupleDesc(1, false);
--- 1146,1152 ----
  		(PQresultStatus(res) != PGRES_COMMAND_OK &&
  		 PQresultStatus(res) != PGRES_TUPLES_OK))
  	{
! 		dblink_res_error(conname, res, "could not execute command", fail);
  
  		/* need a tuple descriptor representing one TEXT column */
  		tupdesc = CreateTemplateTupleDesc(1, false);
***************
*** 1195,1201 ****
  		 * result tuple
  		 */
  		sql_cmd_status = cstring_to_text("ERROR");
- 
  	}
  	else if (PQresultStatus(res) == PGRES_COMMAND_OK)
  	{
--- 1158,1163 ----
***************
*** 2288,2290 ****
--- 2250,2303 ----
  		}
  	}
  }
+ 
+ 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);
+ 	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);
+ 	int			sqlstate;
+ 	char	   *message_primary;
+ 	char	   *message_detail;
+ 	char	   *message_hint;
+ 	char	   *message_context;
+ 	const char *dblink_context_conname = "unnamed";
+ 
+ 	if (fail)
+ 		level = ERROR;
+ 	else
+ 		level = NOTICE;
+ 
+ 	if (pg_diag_sqlstate)
+ 		sqlstate = MAKE_SQLSTATE(pg_diag_sqlstate[0],
+ 								 pg_diag_sqlstate[1],
+ 								 pg_diag_sqlstate[2],
+ 								 pg_diag_sqlstate[3],
+ 								 pg_diag_sqlstate[4]);
+ 	else
+ 		sqlstate = ERRCODE_CONNECTION_FAILURE;
+ 
+ 	xpstrdup(message_primary, pg_diag_message_primary);
+ 	xpstrdup(message_detail, pg_diag_message_detail);
+ 	xpstrdup(message_hint, pg_diag_message_hint);
+ 	xpstrdup(message_context, pg_diag_context);
+ 
+ 	if (res)
+ 		PQclear(res);
+ 
+ 	if (conname)
+ 		dblink_context_conname = conname;
+ 
+ 	ereport(level,
+ 		(errcode(sqlstate),
+ 		 message_primary ? errmsg("%s", message_primary) : errmsg("unknown error"),
+ 		 message_detail ? errdetail("%s", message_detail) : 0,
+ 		 message_hint ? errhint("%s", message_hint) : 0,
+ 		 message_context ? errcontext("%s", message_context) : 0,
+ 		 errcontext("Error occurred on dblink connection named \"%s\": %s.",
+ 					dblink_context_conname, dblink_context_msg)));
+ }
Index: expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.23
diff -c -r1.23 dblink.out
*** expected/dblink.out	6 Apr 2008 16:54:48 -0000	1.23
--- expected/dblink.out	1 Jun 2008 21:59:11 -0000
***************
*** 125,133 ****
  
  -- open a cursor with bad SQL and fail_on_error set to false
  SELECT dblink_open('rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE:  sql error
! DETAIL:  ERROR:  relation "foobar" does not exist
! 
   dblink_open 
  -------------
   ERROR
--- 125,132 ----
  
  -- open a cursor with bad SQL and fail_on_error set to false
  SELECT dblink_open('rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE:  relation "foobar" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not open cursor.
   dblink_open 
  -------------
   ERROR
***************
*** 194,202 ****
  -- intentionally botch a fetch
  SELECT *
  FROM dblink_fetch('rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE:  sql error
! DETAIL:  ERROR:  cursor "rmt_foobar_cursor" does not exist
! 
   a | b | c 
  ---+---+---
  (0 rows)
--- 193,200 ----
  -- intentionally botch a fetch
  SELECT *
  FROM dblink_fetch('rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE:  cursor "rmt_foobar_cursor" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not fetch from cursor.
   a | b | c 
  ---+---+---
  (0 rows)
***************
*** 210,218 ****
  
  -- close the wrong cursor
  SELECT dblink_close('rmt_foobar_cursor',false);
! NOTICE:  sql error
! DETAIL:  ERROR:  cursor "rmt_foobar_cursor" does not exist
! 
   dblink_close 
  --------------
   ERROR
--- 208,215 ----
  
  -- close the wrong cursor
  SELECT dblink_close('rmt_foobar_cursor',false);
! NOTICE:  cursor "rmt_foobar_cursor" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not close cursor.
   dblink_close 
  --------------
   ERROR
***************
*** 221,235 ****
  -- should generate 'cursor "rmt_foo_cursor" not found' error
  SELECT *
  FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR:  sql error
! DETAIL:  ERROR:  cursor "rmt_foo_cursor" does not exist
! 
  -- this time, 'cursor "rmt_foo_cursor" not found' as a notice
  SELECT *
  FROM dblink_fetch('rmt_foo_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE:  sql error
! DETAIL:  ERROR:  cursor "rmt_foo_cursor" does not exist
! 
   a | b | c 
  ---+---+---
  (0 rows)
--- 218,230 ----
  -- should generate 'cursor "rmt_foo_cursor" not found' error
  SELECT *
  FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR:  cursor "rmt_foo_cursor" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not fetch from cursor.
  -- this time, 'cursor "rmt_foo_cursor" not found' as a notice
  SELECT *
  FROM dblink_fetch('rmt_foo_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE:  cursor "rmt_foo_cursor" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not fetch from cursor.
   a | b | c 
  ---+---+---
  (0 rows)
***************
*** 291,299 ****
  -- bad remote select
  SELECT *
  FROM dblink('SELECT * FROM foobar',false) AS t(a int, b text, c text[]);
! NOTICE:  sql error
! DETAIL:  ERROR:  relation "foobar" does not exist
! 
   a | b | c 
  ---+---+---
  (0 rows)
--- 286,293 ----
  -- bad remote select
  SELECT *
  FROM dblink('SELECT * FROM foobar',false) AS t(a int, b text, c text[]);
! NOTICE:  relation "foobar" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not execute query.
   a | b | c 
  ---+---+---
  (0 rows)
***************
*** 316,324 ****
  
  -- botch a change to some other data
  SELECT dblink_exec('UPDATE foobar SET f3[2] = ''b99'' WHERE f1 = 11',false);
! NOTICE:  sql error
! DETAIL:  ERROR:  relation "foobar" does not exist
! 
   dblink_exec 
  -------------
   ERROR
--- 310,317 ----
  
  -- botch a change to some other data
  SELECT dblink_exec('UPDATE foobar SET f3[2] = ''b99'' WHERE f1 = 11',false);
! NOTICE:  relation "foobar" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not execute command.
   dblink_exec 
  -------------
   ERROR
***************
*** 378,386 ****
  SELECT *
  FROM dblink('myconn','SELECT * FROM foobar',false) AS t(a int, b text, c text[])
  WHERE t.a > 7;
! NOTICE:  sql error
! DETAIL:  ERROR:  relation "foobar" does not exist
! 
   a | b | c 
  ---+---+---
  (0 rows)
--- 371,378 ----
  SELECT *
  FROM dblink('myconn','SELECT * FROM foobar',false) AS t(a int, b text, c text[])
  WHERE t.a > 7;
! NOTICE:  relation "foobar" does not exist
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not execute query.
   a | b | c 
  ---+---+---
  (0 rows)
***************
*** 416,424 ****
  
  -- open a cursor incorrectly
  SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE:  sql error
! DETAIL:  ERROR:  relation "foobar" does not exist
! 
   dblink_open 
  -------------
   ERROR
--- 408,415 ----
  
  -- open a cursor incorrectly
  SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foobar',false);
! NOTICE:  relation "foobar" does not exist
! CONTEXT:  Error occurred on dblink connection named "myconn": could not open cursor.
   dblink_open 
  -------------
   ERROR
***************
*** 503,511 ****
  
  -- this should fail because there is no open transaction
  SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');
! ERROR:  sql error
! DETAIL:  ERROR:  DECLARE CURSOR can only be used in transaction blocks
! 
  -- reset remote transaction state
  SELECT dblink_exec('myconn','ABORT');
   dblink_exec 
--- 494,501 ----
  
  -- this should fail because there is no open transaction
  SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo');
! ERROR:  DECLARE CURSOR can only be used in transaction blocks
! CONTEXT:  Error occurred on dblink connection named "unnamed": could not execute command.
  -- reset remote transaction state
  SELECT dblink_exec('myconn','ABORT');
   dblink_exec 
***************
*** 554,562 ****
  -- fetch some data incorrectly
  SELECT *
  FROM dblink_fetch('myconn','rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE:  sql error
! DETAIL:  ERROR:  cursor "rmt_foobar_cursor" does not exist
! 
   a | b | c 
  ---+---+---
  (0 rows)
--- 544,551 ----
  -- fetch some data incorrectly
  SELECT *
  FROM dblink_fetch('myconn','rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]);
! NOTICE:  cursor "rmt_foobar_cursor" does not exist
! CONTEXT:  Error occurred on dblink connection named "myconn": could not fetch from cursor.
   a | b | c 
  ---+---+---
  (0 rows)
***************
*** 571,579 ****
  -- should generate 'cursor "rmt_foo_cursor" not found' error
  SELECT *
  FROM dblink_fetch('myconn','rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR:  sql error
! DETAIL:  ERROR:  cursor "rmt_foo_cursor" does not exist
! 
  -- close the named persistent connection
  SELECT dblink_disconnect('myconn');
   dblink_disconnect 
--- 560,567 ----
  -- should generate 'cursor "rmt_foo_cursor" not found' error
  SELECT *
  FROM dblink_fetch('myconn','rmt_foo_cursor',4) AS t(a int, b text, c text[]);
! ERROR:  cursor "rmt_foo_cursor" does not exist
! CONTEXT:  Error occurred on dblink connection named "myconn": could not fetch from cursor.
  -- close the named persistent connection
  SELECT dblink_disconnect('myconn');
   dblink_disconnect
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.