From 15fd2f79b85b6ee9e8165cd353e922500c907c87 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 5 Feb 2010 18:12:38 +0000
Subject: [PATCH] - 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]

---
 CHANGES                       |  9 +++++++
 lib/sqlalchemy/orm/session.py |  6 ++---
 test/orm/test_transaction.py  | 44 +++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/CHANGES b/CHANGES
index fe0be1d4b9..faf2ee2c40 100644
--- 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 d15fbbc1a7..0e5e939b13 100644
--- 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 51b345cebd..7022a9faa1 100644
--- 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()
+
         
         
 
-- 
2.47.3