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 */
---------------------------(end of broadcast)--------------------------- TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
