From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Joe Conway <mail(at)joeconway(dot)com> |
Cc: | "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>, Jan Wieck <JanWieck(at)Yahoo(dot)com> |
Subject: | Re: Problem with dblink |
Date: | 2003-11-30 19:10:01 |
Message-ID: | 25252.1070219401@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Joe Conway <mail(at)joeconway(dot)com> 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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2003-11-30 19:34:34 | Re: export FUNC_MAX_ARGS as a read-only GUC variable (was: [GENERAL] SELECT Question) |
Previous Message | Peter Eisentraut | 2003-11-30 19:08:29 | Re: export FUNC_MAX_ARGS as a read-only GUC variable (was: |