Re: Removing savepointLevel from TransactionState

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Removing savepointLevel from TransactionState
Date: 2011-09-29 12:10:25
Message-ID: 1317297307-sup-7945@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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 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?

This is the patch I submitted that introduced that bit:
http://archives.postgresql.org/pgsql-patches/2004-07/msg00292.php
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.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2011-09-29 12:21:32 Re: [REVIEW] pg_last_xact_insert_timestamp
Previous Message Bruce Momjian 2011-09-29 11:52:54 Re: pg_upgrade - add config directory setting