- [bug] Fixed bug where "merge" cascade could

mis-interpret an unloaded attribute, if the
load_on_pending flag were used with
relationship().  Thanks to Kent Bower
for tests.  [ticket:2374]

diff --git a/CHANGES b/CHANGES
index 85a705ea73888e4828bff8fa70121257a7f07b07..9c86e50a81037b1b76f19ff67a4eb3306fc849ad 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -73,6 +73,12 @@ CHANGES
     needed ORM attribute access within 
     __eq__() or similar. [ticket:2362]
 
+  - [bug] Fixed bug where "merge" cascade could
+    mis-interpret an unloaded attribute, if the
+    load_on_pending flag were used with 
+    relationship().  Thanks to Kent Bower
+    for tests.  [ticket:2374]
+
 - sql
   - [feature] Added "false()" and "true()" expression
     constructs to sqlalchemy.sql namespace, though

diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index 757deffef01dfff89096b9764f7fcb14143d8e4f..3cd0f15eb14a35d46f0bed0ca57cd46b7427a7be 100644 (file)
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -66,7 +66,10 @@ def track_cascade_events(descriptor, prop):
                     not sess._contains_state(newvalue_state):
                     sess._save_or_update_state(newvalue_state)
 
-            if oldvalue is not None and prop.cascade.delete_orphan:
+            if oldvalue is not None and \
+                oldvalue is not attributes.PASSIVE_NO_RESULT and \
+                prop.cascade.delete_orphan:
+                # possible to reach here with attributes.NEVER_SET ?
                 oldvalue_state = attributes.instance_state(oldvalue)
 
                 if oldvalue_state in sess._new and \

diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py
index 511ab19eae0b68a018775aa7350ab84d3225cd77..f7d9a4d3139323d50342aaf575f98ef42510696e 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -1254,8 +1254,6 @@ class MutableMergeTest(fixtures.MappedTest):
         d3 = sess.merge(d2)
         eq_(d3.data, ["this", "is", "another", "list"])
 
-
-
 class CompositeNullPksTest(fixtures.MappedTest):
     @classmethod
     def define_tables(cls, metadata):
@@ -1293,4 +1291,54 @@ class CompositeNullPksTest(fixtures.MappedTest):
             return sess.merge(d1)
         self.assert_sql_count(testing.db, go, 0)
 
+class LoadOnPendingTest(fixtures.MappedTest):
+    """Test interaction of merge() with load_on_pending relationships"""
+    @classmethod
+    def define_tables(cls, metadata):
+        rocks_table = Table("rocks", metadata,
+            Column("id", Integer, primary_key=True),
+            Column("description", String(10)),
+        )
+        bugs_table = Table("bugs", metadata,
+            Column("id", Integer, primary_key=True),
+            Column("rockid", Integer, ForeignKey('rocks.id')),
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class Rock(cls.Basic, fixtures.ComparableEntity):
+            pass
+        class Bug(cls.Basic, fixtures.ComparableEntity):
+            pass
 
+    def _setup_delete_orphan_o2o(self):
+        mapper(self.classes.Rock, self.tables.rocks,
+            properties={'bug': relationship(self.classes.Bug,
+                            cascade='all,delete-orphan',
+                            load_on_pending=True,
+                            uselist=False)
+                        })
+        mapper(self.classes.Bug, self.tables.bugs)
+        self.sess = sessionmaker()()
+
+    def _merge_delete_orphan_o2o_with(self, bug):
+        # create a transient rock with passed bug
+        r = self.classes.Rock(id=0, description='moldy')
+        r.bug = bug
+        m = self.sess.merge(r)
+        # we've already passed ticket #2374 problem since merge() returned, 
+        # but for good measure:
+        assert m is not r
+        eq_(m,r)
+
+    def test_merge_delete_orphan_o2o_none(self):
+        """one to one delete_orphan relationships marked load_on_pending should
+        be able to merge() with attribute None"""
+        self._setup_delete_orphan_o2o()
+        self._merge_delete_orphan_o2o_with(None)
+
+    def test_merge_delete_orphan_o2o(self):
+        """one to one delete_orphan relationships marked load_on_pending should
+        be able to merge()"""
+        self._setup_delete_orphan_o2o()
+        self._merge_delete_orphan_o2o_with(self.classes.Bug(id=1))