Re: SET TRANSACTION not compliant with SQL:2003

The SQL:2003 standard definition of SET TRANSACTION differs in major
ways from PostgreSQL's, which produces some interesting behaviour.

We currently claim conformance, though this is not accurate.

...
<SQL2003>
If a <set transaction statement> that does not specify LOCAL is
executed, then
Case:
i) If an SQL-transaction is currently active, then an exception
condition is raised: invalid transaction
state — active SQL-transaction.
</SQL2003>
...
<SQL2003>
Case:
a) If LOCAL is not specified, then let TXN be the next SQL-transaction
for the SQL-agent.
b) Otherwise, let TXN be the branch of the active SQL-transaction at the
current SQL-connection.
</SQL2003>

The standard behaviour is that SET TRANSACTION defines the mode used in
the *next* transaction, not the current one. We should allow this
meaning, since programs written to spec will act differently with the
current implementation. We currently only change the *current*
transaction. Executing within the current transaction is supposed to
throw an error; that's probably too late to change, but the standard
does give some clues for other errors.

Proposed changes:

1. Solo SET TRANSACTION statements produce no WARNING, nor do anything.
This isn't the way the SQL:2003 standard specifies it should work.
We should take the values from SET TRANSACTION and apply them to the
*next* transaction:
- these will apply to next TXN, unless specifically overridden during
the START TRANSACTION command
- these values apply for one transaction only, after which we revert
back to the session default.

2. Duplicate calls to SET TRANSACTION are allowed within a transaction.
=> Should be ERROR: Transaction mode already set.

postgres=# begin;
BEGIN
postgres=# set transaction read only;
SET
postgres=# set transaction read only;
SET
postgres=# commit;
COMMIT

3. Multiple conflicting calls to SET TRANSACTION are allowed within a
transaction.
=> Should be ERROR: Transaction mode already set.

postgres=# begin;
BEGIN
postgres=# set transaction isolation level read committed;
SET
postgres=# set transaction isolation level serializable;
SET
postgres=# commit;
COMMIT

4. SET TRANSACTION can be called after a SAVEPOINT, i.e. it can be
called in a subtransaction.
=> Should be ERROR: SET TRANSACTION must not be called in a
subtransaction.
(Calling SET TRANSACTION ISOLATION LEVEL already throws this error, so
change should be small)

5. The standard uses the keyword LOCAL like this:
SET LOCAL TRANSACTION ...
which in this context means the part of a distributed (two-phased)
commit on this database.
We should accept, but ignore this keyword.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com

Tom's comment on this from the patch queue is that the standard assume
autocommit off, which affect some of your analysis below.

---------------------------------------------------------------------------

Simon Riggs wrote:
> The SQL:2003 standard definition of SET TRANSACTION differs in major
> ways from PostgreSQL's, which produces some interesting behaviour.
>
> We currently claim conformance, though this is not accurate.
>
> ...
> <SQL2003>
> If a <set transaction statement> that does not specify LOCAL is
> executed, then
> Case:
> i) If an SQL-transaction is currently active, then an exception
> condition is raised: invalid transaction
> state ? active SQL-transaction.
> </SQL2003>
> ...
> <SQL2003>
> Case:
> a) If LOCAL is not specified, then let TXN be the next SQL-transaction
> for the SQL-agent.
> b) Otherwise, let TXN be the branch of the active SQL-transaction at the
> current SQL-connection.
> </SQL2003>
>
> The standard behaviour is that SET TRANSACTION defines the mode used in
> the *next* transaction, not the current one. We should allow this
> meaning, since programs written to spec will act differently with the
> current implementation. We currently only change the *current*
> transaction. Executing within the current transaction is supposed to
> throw an error; that's probably too late to change, but the standard
> does give some clues for other errors.
>
> Proposed changes:
>
> 1. Solo SET TRANSACTION statements produce no WARNING, nor do anything.
> This isn't the way the SQL:2003 standard specifies it should work.
> We should take the values from SET TRANSACTION and apply them to the
> *next* transaction:
> - these will apply to next TXN, unless specifically overridden during
> the START TRANSACTION command
> - these values apply for one transaction only, after which we revert
> back to the session default.
>
> 2. Duplicate calls to SET TRANSACTION are allowed within a transaction.
> => Should be ERROR: Transaction mode already set.
>
> postgres=# begin;
> BEGIN
> postgres=# set transaction read only;
> SET
> postgres=# set transaction read only;
> SET
> postgres=# commit;
> COMMIT
>
> 3. Multiple conflicting calls to SET TRANSACTION are allowed within a
> transaction.
> => Should be ERROR: Transaction mode already set.
>
> postgres=# begin;
> BEGIN
> postgres=# set transaction isolation level read committed;
> SET
> postgres=# set transaction isolation level serializable;
> SET
> postgres=# commit;
> COMMIT
>
> 4. SET TRANSACTION can be called after a SAVEPOINT, i.e. it can be
> called in a subtransaction.
> => Should be ERROR: SET TRANSACTION must not be called in a
> subtransaction.
> (Calling SET TRANSACTION ISOLATION LEVEL already throws this error, so
> change should be small)
>
> 5. The standard uses the keyword LOCAL like this:
> SET LOCAL TRANSACTION ...
> which in this context means the part of a distributed (two-phased)
> commit on this database.
> We should accept, but ignore this keyword.
>
> --
> Simon Riggs
> 2ndQuadrant http://www.2ndQuadrant.com
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Wed, 2008-03-12 at 15:51 -0400, Bruce Momjian wrote:
> Tom's comment on this from the patch queue is that the standard assume
> autocommit off, which affect some of your analysis below.

This isn't an important area for me, but I don't think we follow the
standard in the way we do it now and we should at least note that.

The standard specifically refers to "the next transaction", so the
setting of autocommit is irrelevant here.

> Simon Riggs wrote:
> > The SQL:2003 standard definition of SET TRANSACTION differs in major
> > ways from PostgreSQL's, which produces some interesting behaviour.
> >
> > We currently claim conformance, though this is not accurate.
> >
> > ...
> > <SQL2003>
> > If a <set transaction statement> that does not specify LOCAL is
> > executed, then
> > Case:
> > i) If an SQL-transaction is currently active, then an exception
> > condition is raised: invalid transaction
> > state ? active SQL-transaction.
> > </SQL2003>
> > ...
> > <SQL2003>
> > Case:
> > a) If LOCAL is not specified, then let TXN be the next SQL-transaction
> > for the SQL-agent.
> > b) Otherwise, let TXN be the branch of the active SQL-transaction at the
> > current SQL-connection.
> > </SQL2003>
> >
> > The standard behaviour is that SET TRANSACTION defines the mode used in
> > the *next* transaction, not the current one. We should allow this
> > meaning, since programs written to spec will act differently with the
> > current implementation. We currently only change the *current*
> > transaction. Executing within the current transaction is supposed to
> > throw an error; that's probably too late to change, but the standard
> > does give some clues for other errors.
> >
> > Proposed changes:
> >
> > 1. Solo SET TRANSACTION statements produce no WARNING, nor do anything.
> > This isn't the way the SQL:2003 standard specifies it should work.
> > We should take the values from SET TRANSACTION and apply them to the
> > *next* transaction:
> > - these will apply to next TXN, unless specifically overridden during
> > the START TRANSACTION command
> > - these values apply for one transaction only, after which we revert
> > back to the session default.
> >
> > 2. Duplicate calls to SET TRANSACTION are allowed within a transaction.
> > => Should be ERROR: Transaction mode already set.
> >
> > postgres=# begin;
> > BEGIN
> > postgres=# set transaction read only;
> > SET
> > postgres=# set transaction read only;
> > SET
> > postgres=# commit;
> > COMMIT
> >
> > 3. Multiple conflicting calls to SET TRANSACTION are allowed within a
> > transaction.
> > => Should be ERROR: Transaction mode already set.
> >
> > postgres=# begin;
> > BEGIN
> > postgres=# set transaction isolation level read committed;
> > SET
> > postgres=# set transaction isolation level serializable;
> > SET
> > postgres=# commit;
> > COMMIT
> >
> > 4. SET TRANSACTION can be called after a SAVEPOINT, i.e. it can be
> > called in a subtransaction.
> > => Should be ERROR: SET TRANSACTION must not be called in a
> > subtransaction.
> > (Calling SET TRANSACTION ISOLATION LEVEL already throws this error, so
> > change should be small)
> >
> > 5. The standard uses the keyword LOCAL like this:
> > SET LOCAL TRANSACTION ...
> > which in this context means the part of a distributed (two-phased)
> > commit on this database.
> > We should accept, but ignore this keyword.
> >
> > --
> > Simon Riggs
> > 2ndQuadrant http://www.2ndQuadrant.com
> >
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 6: explain analyze is your friend
>
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com

PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk

[ back to this patch ]

Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> The SQL:2003 standard definition of SET TRANSACTION differs in major
> ways from PostgreSQL's, which produces some interesting behaviour.

> We currently claim conformance, though this is not accurate.

I'm still of the opinion that this analysis is based on a faulty
analogy.

The standard's discussion is based on autocommit-off behavior.
The expected usage pattern, starting from a not-in-a-transaction
state, is

set transaction something-or-other;
sql-command;
sql-command;
...
commit;

Because we are non-compliant in having autocommit-on behavior, the
expected usage pattern for us is

begin;
set transaction something-or-other;
sql-command;
sql-command;
...
commit;

Simon's proposed patch would allow

set transaction something-or-other;
begin;
sql-command;
sql-command;
...
commit;

but there is no precedent in either the standard or our historical
behavior for supporting that, and I don't think it's useful enough
to justify introducing a potential backward-compatibility problem.

I believe the reason the spec is written in the particular way that
it is is that they wanted to allow, e.g.,

set transaction isolation level serializable;
set transaction read only;
sql-command;
sql-command;
...
commit;

and if SET TRANSACTION actually started a transaction then the second of
these would have to fail. So they're forced into pretending that SET
TRANSACTION should be a non-transaction-starting command that affects
the next transaction. (BTW, they had ample reason to regret that
decision when they added 2PC, and were forced into the ungraceful
position of requiring mutually contradictory behavior between the two
cases.) We arrive at the same behavior (modulo BEGIN) in a different
way, but the user-visible result is the same.

So I'm of the opinion that there's no good reason to change either our
code or our docs. The standard-incompatibility is with BEGIN, not
SET TRANSACTION, and it's already documented.

regards, tom lane

PS: the proposed patch is buggy as can be anyway: it applies the change
even if !doit, and it causes START TRANSACTION ISOLATION LEVEL xxx
to affect not only the current but the next transaction, which surely
cannot be justified by any reading of the spec ;-)

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> I believe the reason the spec is written in the particular way that
> it is is that they wanted to allow, e.g.,
>
> set transaction isolation level serializable;
> set transaction read only;
> sql-command;
> sql-command;
> ...
> commit;

So that works currently. I think you're right that the spec has to be read
assuming autocommit off.

postgres=# \set AUTOCOMMIT off
postgres=# set transaction isolation level serializable;
SET
postgres=# set transaction read only;
SET
postgres=# create table foo (i integer);
ERROR: transaction is read-only
postgres=# rollback;
ROLLBACK
postgres=# set transaction read only;
SET
postgres=# set transaction isolation level serializable;
SET
postgres=# create table i (integer i);
ERROR: transaction is read-only

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

Tom wrote:
> So I'm of the opinion that there's no good reason to change either our
> code or our docs. The standard-incompatibility is with BEGIN, not
> SET TRANSACTION, and it's already documented.

Yes.

> PS: the proposed patch is buggy as can be anyway: it applies the
change
> even if !doit, and it causes START TRANSACTION ISOLATION LEVEL xxx
> to affect not only the current but the next transaction, which surely
> cannot be justified by any reading of the spec ;-)

In IBM Informix the command SET TRANSACTION ISOLATION LEVEL xxx,
returns an error when issued outside a BEGIN WORK -- COMMIT transaction
block.

set transaction isolation level read uncommitted;

255: Not in transaction.

In their latest docs they state:
"The SET TRANSACTION statement complies with ANSI SQL-92."

So I agree that there is no need to change what we have.

Andreas

On Tue, 2008-04-08 at 20:41 -0400, Tom Lane wrote:

> So I'm of the opinion that there's no good reason to change either our
> code or our docs. The standard-incompatibility is with BEGIN, not
> SET TRANSACTION, and it's already documented.

That's a good case. Patch withdrawn.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com