Re: Problem with dblink

>>Tom Lane wrote:
>>>(More generally, I wonder if AtEOXact_SPI oughtn't be fixed to emit
>>>a WARNING if the SPI stack isn't empty, except in the error case.
>>>Neglecting SPI_finish is analogous to a resource leak, and we have
>>>code in place to warn about other sorts of leaks.)

Is the attached what you had in mind? The original problem function 
would now look like:

SELECT dblink_build_sql_insert('foo','1 2',2,'{"0", "a"}','{"99", "xyz"}');
WARNING:  freeing non-empty SPI stack
HINT:  Check for missing "SPI_finish" calls
                   dblink_build_sql_insert
-----------------------------------------------------------
  INSERT INTO foo(f1,f2,f3) VALUES('99','xyz','{a0,b0,c0}')
(1 row)

Joe

Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.157
diff -c -r1.157 xact.c
*** src/backend/access/transam/xact.c	29 Nov 2003 19:51:40 -0000	1.157
--- src/backend/access/transam/xact.c	29 Nov 2003 23:57:09 -0000
***************
*** 977,983 ****
  
  	CallEOXactCallbacks(true);
  	AtEOXact_GUC(true);
! 	AtEOXact_SPI();
  	AtEOXact_gist();
  	AtEOXact_hash();
  	AtEOXact_nbtree();
--- 977,983 ----
  
  	CallEOXactCallbacks(true);
  	AtEOXact_GUC(true);
! 	AtEOXact_SPI(false);
  	AtEOXact_gist();
  	AtEOXact_hash();
  	AtEOXact_nbtree();
***************
*** 1087,1093 ****
  
  	CallEOXactCallbacks(false);
  	AtEOXact_GUC(false);
! 	AtEOXact_SPI();
  	AtEOXact_gist();
  	AtEOXact_hash();
  	AtEOXact_nbtree();
--- 1087,1093 ----
  
  	CallEOXactCallbacks(false);
  	AtEOXact_GUC(false);
! 	AtEOXact_SPI(true);
  	AtEOXact_gist();
  	AtEOXact_hash();
  	AtEOXact_nbtree();
Index: src/backend/executor/spi.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/executor/spi.c,v
retrieving revision 1.108
diff -c -r1.108 spi.c
*** src/backend/executor/spi.c	29 Nov 2003 19:51:48 -0000	1.108
--- src/backend/executor/spi.c	29 Nov 2003 23:57:09 -0000
***************
*** 177,191 ****
   * Clean up SPI state at transaction commit or abort (we don't care which).
   */
  void
! AtEOXact_SPI(void)
  {
  	/*
  	 * Note that memory contexts belonging to SPI stack entries will be
  	 * freed automatically, so we can ignore them here.  We just need to
  	 * restore our static variables to initial state.
  	 */
! 	if (_SPI_stack != NULL)		/* there was abort */
  		free(_SPI_stack);
  	_SPI_current = _SPI_stack = NULL;
  	_SPI_connected = _SPI_curid = -1;
  	SPI_processed = 0;
--- 177,199 ----
   * Clean up SPI state at transaction commit or abort (we don't care which).
   */
  void
! AtEOXact_SPI(bool isAbort)
  {
  	/*
  	 * Note that memory contexts belonging to SPI stack entries will be
  	 * freed automatically, so we can ignore them here.  We just need to
  	 * restore our static variables to initial state.
  	 */
! 	if (_SPI_stack != NULL)
! 	{
  		free(_SPI_stack);
+ 		if (!isAbort)
+ 			ereport(WARNING,
+ 					(errcode(ERRCODE_WARNING),
+ 					 errmsg("freeing non-empty SPI stack"),
+ 					 errhint("Check for missing \"SPI_finish\" calls")));
+ 	}
+ 
  	_SPI_current = _SPI_stack = NULL;
  	_SPI_connected = _SPI_curid = -1;
  	SPI_processed = 0;
Index: src/include/executor/spi.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/executor/spi.h,v
retrieving revision 1.40
diff -c -r1.40 spi.h
*** src/include/executor/spi.h	29 Nov 2003 22:41:01 -0000	1.40
--- src/include/executor/spi.h	29 Nov 2003 23:57:09 -0000
***************
*** 116,121 ****
  extern void SPI_cursor_move(Portal portal, bool forward, int count);
  extern void SPI_cursor_close(Portal portal);
  
! extern void AtEOXact_SPI(void);
  
  #endif   /* SPI_H */
--- 116,121 ----
  extern void SPI_cursor_move(Portal portal, bool forward, int count);
  extern void SPI_cursor_close(Portal portal);
  
! extern void AtEOXact_SPI(bool isAbort);
  
  #endif   /* SPI_H */