Re: Problem with dblink

Tom Lane wrote:
> 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