From 733d8e593a611b4601ec2ee197ca112250111b64 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 23 Feb 2010 00:35:56 +0000 Subject: [PATCH] - 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. --- CHANGES | 12 ++++ lib/sqlalchemy/orm/properties.py | 6 +- test/orm/test_merge.py | 96 +++++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index 3f73cfe854..a587b3b04b 100644 --- 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 7c605dd8e2..20a1319f8e 100644 --- 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 f8ee559b63..8dc849e01e 100644 --- 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): -- 2.47.3