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])

Reply via email to