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

Joe Conway <[email protected]> Sun, 01 Jun 2008 12:03:41 -0700

Newsgroups	gmane.comp.db.postgresql.devel.patches
Message-ID	<[email protected]>

Tom Lane wrote:
> Joe Conway <[email protected]> writes:
>> Tom Lane wrote:
>>> Yeah, the dblink code should probably try a bit harder to propagate the
>>> original error fields.
> 
>> Sounds reasonable. Do you think this is a bug fix or an 8.4 enhancement? 
> 
> 8.4 enhancement I think, since a behavioral change here could pose a
> compatibility issue for applications.
> 

Here is my proposed patch -- as suggested for cvs tip only.

I haven't been around enough lately to be sure I understand the process 
these days. Should I be posting this to the wiki and waiting for the 
next commit fest, or just apply myself in a day or two assuming no 
objections?

Thanks,

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.1.diff (text/x-patch, 13.1 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 18:50:04 -0000
***************
*** 135,158 ****
  
  #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 \
--- 135,182 ----
  
  #define DBLINK_RES_ERROR(p2) \
  	do { \
! 			char *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); \
  			if (res) \
  				PQclear(res); \
! 			errstart(ERROR, __FILE__, __LINE__, PG_FUNCNAME_MACRO); \
! 			errcode(MAKE_SQLSTATE(sqlstate[0],sqlstate[1],sqlstate[2],sqlstate[3],sqlstate[4])); \
! 			errmsg("%s", pg_diag_message_primary); \
! 			if (pg_diag_message_detail) \
! 				 errdetail("%s", pg_diag_message_detail); \
! 			if (pg_diag_message_hint) \
! 				 errhint("%s", pg_diag_message_hint); \
! 			if (pg_diag_context) \
! 				errcontext("%s", pg_diag_context); \
! 			if (p2) \
! 				errcontext("error on dblink connection: %s", p2); \
! 			errfinish(0); \
  	} while (0)
  
  #define DBLINK_RES_ERROR_AS_NOTICE(p2) \
  	do { \
! 			char *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); \
  			if (res) \
  				PQclear(res); \
! 			errstart(NOTICE, __FILE__, __LINE__, PG_FUNCNAME_MACRO); \
! 			errcode(MAKE_SQLSTATE(sqlstate[0],sqlstate[1],sqlstate[2],sqlstate[3],sqlstate[4])); \
! 			errmsg("%s", pg_diag_message_primary); \
! 			if (pg_diag_message_detail) \
! 				 errdetail("%s", pg_diag_message_detail); \
! 			if (pg_diag_message_hint) \
! 				 errhint("%s", pg_diag_message_hint); \
! 			if (pg_diag_context) \
! 				errcontext("%s", pg_diag_context); \
! 			if (p2) \
! 				errcontext("error on dblink connection: %s", p2); \
! 			errfinish(0); \
  	} while (0)
  
  #define DBLINK_CONN_NOT_AVAIL \
***************
*** 397,406 ****
  	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"));
  		}
  	}
--- 421,430 ----
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
  		if (fail)
! 			DBLINK_RES_ERROR("unable to open cursor");
  		else
  		{
! 			DBLINK_RES_ERROR_AS_NOTICE("unable to open cursor");
  			PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
  		}
  	}
***************
*** 471,480 ****
  	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"));
  		}
  	}
--- 495,504 ----
  	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
  	{
  		if (fail)
! 			DBLINK_RES_ERROR("unable to close cursor");
  		else
  		{
! 			DBLINK_RES_ERROR_AS_NOTICE("unable to close cursor");
  			PG_RETURN_TEXT_P(cstring_to_text("ERROR"));
  		}
  	}
***************
*** 513,519 ****
  	int			call_cntr;
  	int			max_calls;
  	AttInMetadata *attinmeta;
- 	char	   *msg;
  	PGresult   *res = NULL;
  	MemoryContext oldcontext;
  	char	   *conname = NULL;
--- 537,542 ----
***************
*** 591,600 ****
  			 PQresultStatus(res) != PGRES_TUPLES_OK))
  		{
  			if (fail)
! 				DBLINK_RES_ERROR("sql error");
  			else
  			{
! 				DBLINK_RES_ERROR_AS_NOTICE("sql error");
  				SRF_RETURN_DONE(funcctx);
  			}
  		}
--- 614,623 ----
  			 PQresultStatus(res) != PGRES_TUPLES_OK))
  		{
  			if (fail)
! 				DBLINK_RES_ERROR("unable to fetch from cursor");
  			else
  			{
! 				DBLINK_RES_ERROR_AS_NOTICE("unable to fetch from cursor");
  				SRF_RETURN_DONE(funcctx);
  			}
  		}
***************
*** 847,856 ****
  				 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);
--- 870,879 ----
  				 PQresultStatus(res) != PGRES_TUPLES_OK))
  			{
  				if (fail)
! 					DBLINK_RES_ERROR("unable to execute query");
  				else
  				{
! 					DBLINK_RES_ERROR_AS_NOTICE("unable to execute query");
  					if (freeconn)
  						PQfinish(conn);
  					SRF_RETURN_DONE(funcctx);
***************
*** 1181,1189 ****
  		 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);
--- 1204,1212 ----
  		 PQresultStatus(res) != PGRES_TUPLES_OK))
  	{
  		if (fail)
! 			DBLINK_RES_ERROR("unable to execute command");
  		else
! 			DBLINK_RES_ERROR_AS_NOTICE("unable to execute command");
  
  		/* need a tuple descriptor representing one TEXT column */
  		tupdesc = CreateTemplateTupleDesc(1, false);
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 18:53:06 -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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to 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 on dblink connection: unable to fetch from cursor
  -- close the named persistent connection
  SELECT dblink_disconnect('myconn');
   dblink_disconnect