[sqlalchemy] Nested transaction rollback does not undo changes to instances

[Chris Wilson]

Dear SQLAlchemy developers,

I think I've found a problem with SQLAlchemy not rolling back changes to
instances that are committed in an inner nested transaction, when the outer
nested transaction rolls back.

The manual says:

When begin_nested()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.begin_nested>
is called, a flush()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.flush>
is unconditionally issued (regardless of the autoflush setting). This is so
that when a rollback()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.rollback>
occurs, the full state of the session is expired, thus causing all
subsequent attribute/instance access to reference the full state of the
Session
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session>
right before begin_nested()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.begin_nested>was
called.


So I think that if we make a change to an instance, inside a transaction,
and then rollback, we should see the old values in any instances, as if the
transaction never happened. And indeed this appears to work for simple
cases:

from sqlalchemy import Column, Integer, Boolean
from sqlalchemy import create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker

Base = declarative_base()
class Test(Base):
__tablename__ = 'test'
id = Column(Integer, primary_key=True)
value = Column(Boolean)

engine = create_engine('sqlite:///sqlalchemy_example.db')

Base.metadata.create_all(engine)

DBSession = sessionmaker(bind=engine)
session = DBSession(autocommit=True)

session.begin()
test_data = Test(value=False)
session.add(test_data)
session.commit()
print test_data.value, "in the beginning"

try:
with session.begin():
# with session.begin_nested():
test_data.value = True
print test_data.value, "before rollback"
raise ValueError("force a rollback")
except ValueError as e:
print "ValueError caught: {}".format(e)
print test_data.value, "after rollback"


Which, as expected, resets value from True to False during rollback():

False in the beginning

True before rollback

ValueError caught: force a rollback

False after rollback


Note: this example doesn't work with my SQLite install because it doesn't
seem to support savepoints properly, even though it's documented to do so.
I used postgresql to actually run these tests, and created the table as
follows:

CREATE TABLE test (id SERIAL NOT NULL PRIMARY KEY, value BOOLEAN);
GRANT ALL ON TABLE test TO standard;
GRANT ALL ON TABLE test_id_seq TO standard;


It even works if you rollback a nested transaction:

try:
with session.begin():
try:
with session.begin_nested():
test_data.value = True
print test_data.value, "after nested commit, before nested
rollback"
raise ValueError("force a rollback")
except ValueError as e:
print "ValueError caught: {}".format(e)

print test_data.value, "after nested rollback"
assert not test_data.value, "should have been rolled back"

assert session.transaction is not None
raise ValueError("force a rollback")
except ValueError as e:
print "ValueError caught: {}".format(e)
print test_data.value, "after outer rollback"


However, it does NOT work if you roll back a nested transaction that has a
committed nested transaction inside it (differences highlighted):

try:
with session.begin():
try:
with session.begin_nested():
with session.begin_nested():
test_data.value = True
print test_data.value, "after nested commit, before nested
rollback"
raise ValueError("force a rollback")
except ValueError as e:
print "ValueError caught: {}".format(e)

print test_data.value, "after nested rollback"
# assert not test_data.value, "should have been rolled back; this
assertion fails if enabled"

assert session.transaction is not None
raise ValueError("force a rollback")
except ValueError as e:
print "ValueError caught: {}".format(e)

print test_data.value, "after outer rollback"
assert session.transaction is None


Which outputs:

False in the beginning

True after nested commit, before nested rollback

ValueError caught: force a rollback

True after nested rollback

ValueError caught: force a rollback

False after outer rollback


You can see that after the nested transaction rollback, the value on the
instance has not been reset to False, but the outer (final) rollback
successfully resets it.

We spotted this because we run tests in transactions (currently nested
transactions, although now I have to change it to work around this issue),
and when I make a change in a transaction inside a test, subsequent tests
see the modified value of the instance, while the database still has the
old value, so queries return unexpected results. (In particular, I try to
reset a flag on an instance to True at the start of every test, and this
does nothing if SQLAlchemy thinks that the instance already has this value).

As one of my colleagues said: "Put succinctly, this should be ‘False’. It’s
getting confused by the committing of an inner nested transaction then a
rolling back of an outer subtransaction. I have a feeling it might be quite
a tricky one as I believe the ‘dirtyness’ of an object (i.e. whether it
needs refreshing when you rollback) is not tied to which ever
sub-transaction you are in.. After the savepoint ‘commit’ SQLAlchemy might
not consider the object ‘modified’ and thus not in need of a refresh after
the subsequent rollback."

I'm not sure how best to fix it. I think it's connected to these lines in
sqlalchemy.orm.session.py (lines 286-288), which I think are responsible
for undoing state changes:

for s in self.session.identity_map.all_states():
if not dirty_only or s.modified or s in self._dirty:
s._expire(s.dict, self.session.identity_map._modified)

Where dirty_only is set differently for nested an non-nested transactions
(line 434):

self._restore_snapshot(dirty_only=self.nested)

But I don't know how to identify just which objects were modified inside
the transaction and reset their state to the way they were when the
transaction started, because I can't see how _expire() does it. Any ideas?
Is this a bug?

Thanks, Chris.

[Mike Bayer]

Post by Chris Wilson
Dear SQLAlchemy developers,
I think I've found a problem with SQLAlchemy not rolling back changes
to instances that are committed in an inner nested transaction, when
the outer nested transaction rolls back.
When begin_nested()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.begin_nested>
is called, a flush()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.flush>
is unconditionally issued (regardless of the autoflush setting).
This is so that when a rollback()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.rollback>
occurs, the full state of the session is expired, thus causing all
subsequent attribute/instance access to reference the full state
of the Session
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session>
right before begin_nested()
<http://docs.sqlalchemy.org/en/latest/orm/session_api.html#sqlalchemy.orm.session.Session.begin_nested>was
called.
So I think that if we make a change to an instance, inside a
transaction, and then rollback, we should see the old values in any
instances, as if the transaction never happened. And indeed this
from sqlalchemy import Column, Integer, Boolean
from sqlalchemy import create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker
Base = declarative_base()
__tablename__ = 'test'
id = Column(Integer, primary_key=True)
value = Column(Boolean)
engine = create_engine('sqlite:///sqlalchemy_example.db')
Base.metadata.create_all(engine)
DBSession = sessionmaker(bind=engine)
session = DBSession(autocommit=True)
session.begin()
test_data = Test(value=False)
session.add(test_data)
session.commit()
print test_data.value, "in the beginning"
test_data.value = True
print test_data.value, "before rollback"
raise ValueError("force a rollback")
print "ValueError caught: {}".format(e)
print test_data.value, "after rollback"
False in the beginning
True before rollback
ValueError caught: force a rollback
False after rollback
Note: this example doesn't work with my SQLite install because it
doesn't seem to support savepoints properly, even though it's
documented to do so. I used postgresql to actually run these tests,
CREATE TABLE test (id SERIAL NOT NULL PRIMARY KEY, value BOOLEAN);
GRANT ALL ON TABLE test TO standard;
GRANT ALL ON TABLE test_id_seq TO standard;
test_data.value = True
print test_data.value, "after nested commit,
before nested rollback"
raise ValueError("force a rollback")
print "ValueError caught: {}".format(e)
print test_data.value, "after nested rollback"
assert not test_data.value, "should have been rolled back"
assert session.transaction is not None
raise ValueError("force a rollback")
print "ValueError caught: {}".format(e)
print test_data.value, "after outer rollback"
However, it does NOT work if you roll back a nested transaction that
test_data.value = True
print test_data.value, "after nested commit,
before nested rollback"
raise ValueError("force a rollback")
print "ValueError caught: {}".format(e)
print test_data.value, "after nested rollback"
# assert not test_data.value, "should have been rolled
back; this assertion fails if enabled"
assert session.transaction is not None
raise ValueError("force a rollback")
print "ValueError caught: {}".format(e)
print test_data.value, "after outer rollback"
assert session.transaction is None
False in the beginning
True after nested commit, before nested rollback
ValueError caught: force a rollback
True after nested rollback
ValueError caught: force a rollback
False after outer rollback
You can see that after the nested transaction rollback, the value on
the instance has not been reset to False, but the outer (final)
rollback successfully resets it.
We spotted this because we run tests in transactions (currently nested
transactions, although now I have to change it to work around this
issue), and when I make a change in a transaction inside a test,
subsequent tests see the modified value of the instance, while the
database still has the old value, so queries return unexpected
results. (In particular, I try to reset a flag on an instance to True
at the start of every test, and this does nothing if SQLAlchemy thinks
that the instance already has this value).
As one of my colleagues said: "Put succinctly, this should be ‘False’.
It’s getting confused by the committing of an inner nested transaction
then a rolling back of an outer subtransaction. I have a feeling it
might be quite a tricky one as I believe the ‘dirtyness’ of an object
(i.e. whether it needs refreshing when you rollback) is not tied to
which ever sub-transaction you are in.. After the savepoint ‘commit’
SQLAlchemy might not consider the object ‘modified’ and thus not in
need of a refresh after the subsequent rollback."
I'm not sure how best to fix it. I think it's connected to these lines
in sqlalchemy.orm.session.py (lines 286-288), which I think are
s._expire(s.dict, self.session.identity_map._modified)
Where dirty_only is set differently for nested an non-nested
self._restore_snapshot(dirty_only=self.nested)
But I don't know how to identify just which objects were modified
inside the transaction and reset their state to the way they were when
the transaction started, because I can't see how _expire() does it.
Any ideas? Is this a bug?

it's a bug, and I've captured the origin, diagnosis and probable
solution here:
https://bitbucket.org/zzzeek/sqlalchemy/issue/3352/nested-begin_nested-blocks-dont-track

Post by Chris Wilson
Thanks, Chris.
--
You received this message because you are subscribed to the Google
Groups "sqlalchemy" group.
To unsubscribe from this group and stop receiving emails from it, send
Visit this group at http://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.

[Mike Bayer]

Post by Mike Bayer
it's a bug, and I've captured the origin, diagnosis and probable
https://bitbucket.org/zzzeek/sqlalchemy/issue/3352/nested-begin_nested-blocks-dont-track

this issue is fixed for 0.9.10 and 1.0.0b5, you can test now using
either latest master or the rel_0_9 branch.

[Philip Scott]

(Chris's colleague here)

this issue is fixed for 0.9.10 and 1.0.0b5, you can test now using either
latest master or the rel_0_9 branch.

Just wanted to say thank you very much for the prompt fix; I continue to be
amazed at your efforts :)