From: | Joe Conway <mail(at)joeconway(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>, Jan Wieck <JanWieck(at)Yahoo(dot)com> |
Subject: | Re: Problem with dblink |
Date: | 2003-12-01 04:04:12 |
Message-ID: | 3FCABDBC.7000203@joeconway.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
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
Attachment | Content-Type | Size |
---|---|---|
spi-no-finish.2.patch | text/plain | 3.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2003-12-01 04:10:09 | Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions? |
Previous Message | Bruce Momjian | 2003-12-01 03:55:26 | GUC descriptions of SHOW |