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.

-- 
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 an email 
to sqlalchemy+unsubscr...@googlegroups.com.
To post to this group, send email to sqlalchemy@googlegroups.com.
Visit this group at http://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.

Reply via email to