Joe Conway <[EMAIL PROTECTED]> writes: > 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? Approximately. A couple minor stylistic gripes: 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. 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). > ! 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?) regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])