- Fixed bug in session.rollback() which involved not removing

formerly "pending" objects from the session before
re-integrating "deleted" objects, typically occured with
natural primary keys. If there was a primary key conflict
between them, the attach of the deleted would fail
internally. The formerly "pending" objects are now expunged
first. [ticket:1674]

diff --git a/CHANGES b/CHANGES
index fe0be1d4b987fee8b95d9d8e7ea476302972a33b..faf2ee2c40dcc150d048f15abc30f996ef5b72c3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,15 @@
 CHANGES
 =======
 0.6beta2
+- orm
+  - Fixed bug in session.rollback() which involved not removing
+    formerly "pending" objects from the session before
+    re-integrating "deleted" objects, typically occured with
+    natural primary keys. If there was a primary key conflict
+    between them, the attach of the deleted would fail
+    internally. The formerly "pending" objects are now expunged
+    first. [ticket:1674]
+    
 - sql
   - Added math negation operator support, -x.
 

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index d15fbbc1a7cc69d3aca478107eee74a40c8cbf4a..0e5e939b1386cb9ad42344cf45dce261a96ce5de 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -280,14 +280,14 @@ class SessionTransaction(object):
     def _restore_snapshot(self):
         assert self._is_transaction_boundary
 
+        for s in set(self._new).union(self.session._new):
+            self.session._expunge_state(s)
+
         for s in set(self._deleted).union(self.session._deleted):
             self.session._update_impl(s)
 
         assert not self.session._deleted
 
-        for s in set(self._new).union(self.session._new):
-            self.session._expunge_state(s)
-
         for s in self.session.identity_map.all_states():
             _expire_state(s, None, instance_dict=self.session.identity_map)
 

diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py
index 51b345cebd96deb7563c4787dfc82be40d0f7ac0..7022a9faa1decafeeb18c9ad6d9dfe7207851d7e 100644 (file)
--- a/test/orm/test_transaction.py
+++ b/test/orm/test_transaction.py
@@ -3,6 +3,7 @@ from sqlalchemy.test.testing import eq_, assert_raises, assert_raises_message
 from sqlalchemy import *
 from sqlalchemy.orm import attributes
 from sqlalchemy import exc as sa_exc
+from sqlalchemy.orm import exc as orm_exc
 from sqlalchemy.orm import *
 from sqlalchemy.test.util import gc_collect
 from sqlalchemy.test import testing
@@ -23,6 +24,7 @@ class TransactionTest(FixtureTest):
         mapper(Address, addresses)
 
 
+    
 class FixtureDataTest(TransactionTest):
     run_inserts = 'each'
     
@@ -490,6 +492,48 @@ class AutoCommitTest(TransactionTest):
         assert u2 not in sess
         assert u1 in sess
         assert sess.query(User).filter_by(name='ed').one() is u1
+
+class NaturalPKRollbackTest(_base.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('users', metadata,
+            Column('name', String(50), primary_key=True)
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class User(_base.ComparableEntity):
+            pass
+
+    @testing.resolve_artifact_names
+    def test_rollback_recover(self):
+        mapper(User, users)
+
+        session = sessionmaker()()
+
+        u1, u2, u3= \
+            User(name='u1'),\
+            User(name='u2'),\
+            User(name='u3')
+
+        session.add_all([u1, u2, u3])
+
+        session.commit()
+
+        session.delete(u2)
+        u4 = User(name='u2')
+        session.add(u4)
+        session.flush()
+
+        u5 = User(name='u3')
+        session.add(u5)
+        assert_raises(orm_exc.FlushError, session.flush)
+
+        assert u5 not in session
+        assert u2 not in session.deleted
+
+        session.rollback()
+