- session.merge() will not expire existing scalar attributes

on an existing target if the target has a value for that
attribute, even if the incoming merged doesn't have
a value for the attribute.  This prevents unnecessary loads
on existing items.  Will still mark the attr as expired
if the destination doesn't have the attr, though, which
fulfills some contracts of deferred cols.  [ticket:1681]

- session.merge() works with relations that specifically
don't include "merge" in their cascade options - the target
is ignored completely.

diff --git a/CHANGES b/CHANGES
index 3f73cfe854debfa59997563613be0abe948be813..a587b3b04bb68f241c52bffcf91d6d431910ad7c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -19,6 +19,18 @@ CHANGES
     internally. The formerly "pending" objects are now expunged
     first. [ticket:1674]
 
+  - session.merge() will not expire existing scalar attributes
+    on an existing target if the target has a value for that
+    attribute, even if the incoming merged doesn't have
+    a value for the attribute.  This prevents unnecessary loads
+    on existing items.  Will still mark the attr as expired
+    if the destination doesn't have the attr, though, which
+    fulfills some contracts of deferred cols.  [ticket:1681]
+
+  - session.merge() works with relations that specifically
+    don't include "merge" in their cascade options - the target
+    is ignored completely.
+    
   - query.one() no longer applies LIMIT to the query, this to
     ensure that it fully counts all object identities present
     in the result, even in the case where joins may conceal

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 7c605dd8e266b84a70a4e01878015e52dd05ed65..20a1319f8ef5eadb17f414e434f71fffdc7ac521 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -115,8 +115,9 @@ class ColumnProperty(StrategizedProperty):
                 impl = dest_state.get_impl(self.key)
                 impl.set(dest_state, dest_dict, value, None)
         else:
-            dest_state.expire_attributes(dest_dict, [self.key])
-
+            if self.key not in dest_dict:
+                dest_state.expire_attributes(dest_dict, [self.key])
+                
     def get_col_value(self, column, value):
         return value
 
@@ -636,7 +637,6 @@ class RelationProperty(StrategizedProperty):
                     return
 
         if not "merge" in self.cascade:
-            dest_state.expire_attribute(dest_dict, [self.key])
             return
 
         if self.key not in source_dict:

diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py
index f8ee559b631a36f27fb57201c2ae8fb8bbbce5d6..8dc849e01e4b185932b5c7aed48ad07f47a38535 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -241,12 +241,67 @@ class MergeTest(_fixtures.FixtureTest):
     @testing.resolve_artifact_names
     def test_merge_empty_attributes(self):
         mapper(User, dingalings)
-        u1 = User(id=1)
+        
         sess = create_session()
-        sess.merge(u1)
+        
+        # merge empty stuff.  goes in as NULL.
+        # not sure what this was originally trying to 
+        # test.
+        u1 = sess.merge(User(id=1))
         sess.flush()
-        assert u1.address_id is u1.data is None
+        assert u1.data is None
 
+        # save another user with "data"
+        u2 = User(id=2, data="foo")
+        sess.add(u2)
+        sess.flush()
+        
+        # merge User on u2's pk with
+        # no "data".
+        # value isn't whacked from the destination
+        # dict.
+        u3 = sess.merge(User(id=2))
+        eq_(u3.__dict__['data'], "foo")
+        
+        # make a change.
+        u3.data = 'bar'
+        
+        # merge another no-"data" user.
+        # attribute maintains modified state.
+        # (usually autoflush would have happened
+        # here anyway).
+        u4 = sess.merge(User(id=2))
+        eq_(u3.__dict__['data'], "bar")
+
+        sess.flush()
+        # and after the flush.
+        eq_(u3.data, "bar")
+
+        # new row.
+        u5 = User(id=3, data="foo")
+        sess.add(u5)
+        sess.flush()
+        
+        # blow it away from u5, but don't
+        # mark as expired.  so it would just 
+        # be blank.
+        del u5.data
+        
+        # the merge adds expiry to the
+        # attribute so that it loads.
+        # not sure if I like this - it currently is needed
+        # for test_pickled:PickleTest.test_instance_deferred_cols
+        u6 = sess.merge(User(id=3))
+        assert 'data' not in u6.__dict__
+        assert u6.data == "foo"
+
+        # set it to None.  this is actually
+        # a change so gets preserved.
+        u6.data = None
+        u7 = sess.merge(User(id=3))
+        assert u6.__dict__['data'] is None
+        
+        
     @testing.resolve_artifact_names
     def test_merge_irregular_collection(self):
         mapper(User, users, properties={
@@ -358,6 +413,41 @@ class MergeTest(_fixtures.FixtureTest):
         eq_(u2.addresses[1].email_address, 'afafds')
         eq_(on_load.called, 21)
 
+    @testing.resolve_artifact_names
+    def test_no_relation_cascade(self):
+        """test that merge doesn't interfere with a relation()
+           target that specifically doesn't include 'merge' cascade.
+        """
+        mapper(Address, addresses, properties={
+            'user':relation(User, cascade="save-update")
+        })
+        mapper(User, users)
+        sess = create_session()
+        u1 = User(name="fred")
+        a1 = Address(email_address="asdf", user=u1)
+        sess.add(a1)
+        sess.flush()
+        
+        a2 = Address(id=a1.id, email_address="bar", user=User(name="hoho"))
+        a2 = sess.merge(a2)
+        sess.flush()
+        
+        # no expire of the attribute
+        
+        assert a2.__dict__['user'] is u1
+        
+        # merge succeeded
+        eq_(
+            sess.query(Address).all(),
+            [Address(id=a1.id, email_address="bar")]
+        )
+        
+        # didn't touch user
+        eq_(
+            sess.query(User).all(),
+            [User(name="fred")]
+        )
+        
     @testing.resolve_artifact_names
     def test_one_to_many_cascade(self):