1. AFAIR, all the other at-end-of-xact routines that take a flag telling them if it's commit or abort define the flag as isCommit. Having the reverse convention for this one routine is confusing and a recipe for errors, so please make it be isCommit too.
Done.
2. The initial comment for AtEOXact_SPI:
* Clean up SPI state at transaction commit or abort (we don't care which).
is now incorrect and needs to be updated (at least get rid of the parenthetical note).
Done.
! 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")));
+ }
+
While this isn't necessarily wrong, what would probably be a stronger test is to complain if either _SPI_connected or _SPI_curid is not -1. For one thing that would catch missing SPI_pop(). (Jan, any comment about that?)
Based on some simple testing, a warning for (_SPI_connected != -1) seems redundant with (_SPI_stack != NULL) -- i.e. I just get both warnings when SPI_finish() isn't called. And commenting out SPI_pop() causes a "SPI_finish failed" error, at least in the one place it's used (pl_exec.c), so we never get to AtEOXact_SPI(). So for the moment at least I left that last section the same.
OK to commit?
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 1 Dec 2003 03:50:40 -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(true);
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(false);
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 1 Dec 2003 03:50:40 -0000
***************
*** 174,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;
--- 174,199 ----
}
/*
! * Clean up SPI state at transaction commit or abort.
*/
void
! AtEOXact_SPI(bool isCommit)
{
/*
* 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 (isCommit)
+ 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 1 Dec 2003 03:50:43 -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 isCommit);
#endif /* SPI_H */
---------------------------(end of broadcast)--------------------------- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
