Excerpts from Tom Lane's message of jue sep 29 02:11:52 -0300 2011:
> Gurjeet Singh <singh.gurjeet@gmail.com> writes: > > I noticed that the savepointLevel member of TransactionStateData struct is > > 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?
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 implementation.
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 that matter.
Dead code trips the unwary like me, and definitely does not help a maintainer.
Regards, --
Gurjeet Singh EnterpriseDB Corporation The Enterprise PostgreSQL Company
