On Thu, Sep 29, 2011 at 8:10 AM, Alvaro Herrera
> Excerpts from Tom Lane's message of jue sep 29 02:11:52 -0300 2011:
> > Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> writes:
> > > I noticed that the savepointLevel member of TransactionStateData struct
> > > initialized to 0 from TopTransactionStateData, and never incremented or
> > > decremented afterwards.
> > > Since this is a file-local struct I think we can simply get rid of all
> > > usages of this without any risk.
> > ISTM you have detected a bug, not just dead code that should be removed.
> > Surely those tests that throw error on savepointLevel change were
> > meant to do something important?
> This is the patch I submitted that introduced that bit:
> which got committed as cc813fc2b8d9293bbd4d0e0d6a6f3b9cf02fe32f.
> Amusingly, the savepointLevel thing was introduced there; I don't
> remember the details but I think what it was intended to implement is
> some sort of restriction laid out by the SQL standard's spelling of
> savepoint commands.
> ... in fact, SQL 2008 talks about savepoint levels in "4.35.2
> Savepoints". And as far as "Part 2: Foundation" is concerned, I think
> only <routine invocation> can cause the savepoint level to be changed.
> That is, if you have a function that declares itself to have NEW SAVEPOINT
> LEVEL, then that function is not allowed to roll back savepoints that
> were created before it started.
> Now, we already disallow functions from doing this at all; so it seems
> that the missing feature for us is OLD SAVEPOINT LEVEL (which, according
> to the standard, is the default behavior). Since this is not
> implementable on the current SPI abstraction, we cannot do much about
> this. But if we ever have transaction-controlling SPs, then it seems to
> me that we ought to keep this and enable those to use it as appropriate.
I have seen some recent discussions around implementing procedures that
would allow transaction control, but don't know at what stage those
conversations ended. If we are still at hand-waving stage w.r.t SPs then I
would vote for removal of this code and rethink it as part of SP
Having seen some commits after the initial one, that use this variable, ISTM
that we're maintaining a feature we never documented, or implemented for
Dead code trips the unwary like me, and definitely does not help a
The Enterprise PostgreSQL Company
In response to
pgsql-hackers by date
|Next:||From: MauMau||Date: 2011-09-29 13:13:30|
|Subject: Re: Does RelCache/SysCache shrink except when relations are deleted?|
|Previous:||From: Bruce Momjian||Date: 2011-09-29 12:56:09|
|Subject: Re: pg_upgrade - add config directory setting|