From: Mike Bayer Date: Wed, 7 Jul 2010 16:01:02 +0000 (-0400) Subject: - Removed errant many-to-many load in unitofwork X-Git-Tag: rel_0_6_3~21 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=91ad6902c524f7d113dfd568288c0a5240ba9499;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Removed errant many-to-many load in unitofwork which triggered unnecessarily on expired/unloaded collections. This load now takes place only if passive_updates is False and the parent primary key has changed, or if passive_deletes is False and a delete of the parent has occurred. [ticket:1845] --- diff --git a/CHANGES b/CHANGES index 11ae0e5751..6f188b10e0 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,17 @@ ======= CHANGES ======= +0.6.3 +===== +- orm + - Removed errant many-to-many load in unitofwork + which triggered unnecessarily on expired/unloaded + collections. This load now takes place only if + passive_updates is False and the parent primary + key has changed, or if passive_deletes is False + and a delete of the parent has occurred. + [ticket:1845] + 0.6.2 ===== - orm diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index bd4473ef79..baa7af645e 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -871,6 +871,9 @@ class ManyToManyDP(DependencyProcessor): def presort_deletes(self, uowcommit, states): if not self.passive_deletes: + # if no passive deletes, load history on + # the collection, so that prop_has_changes() + # returns True for state in states: history = uowcommit.get_attribute_history( state, @@ -878,16 +881,22 @@ class ManyToManyDP(DependencyProcessor): passive=self.passive_deletes) def presort_saves(self, uowcommit, states): - for state in states: - history = uowcommit.get_attribute_history( - state, - self.key, - False) + if not self.passive_updates: + # if no passive updates, load history on + # each collection where parent has changed PK, + # so that prop_has_changes() returns True + for state in states: + if self._pks_changed(uowcommit, state): + history = uowcommit.get_attribute_history( + state, + self.key, + False) - if not self.cascade.delete_orphan: return - + + # check for child items removed from the collection + # if delete_orphan check is turned on. for state in states: history = uowcommit.get_attribute_history( state, @@ -911,6 +920,8 @@ class ManyToManyDP(DependencyProcessor): processed = self._get_reversed_processed_set(uowcommit) tmp = set() for state in states: + # this history should be cached already, as + # we loaded it in preprocess_deletes history = uowcommit.get_attribute_history( state, self.key, @@ -947,7 +958,10 @@ class ManyToManyDP(DependencyProcessor): tmp = set() for state in states: - history = uowcommit.get_attribute_history(state, self.key) + need_cascade_pks = not self.passive_updates and \ + self._pks_changed(uowcommit, state) + history = uowcommit.get_attribute_history(state, self.key, + passive=not need_cascade_pks) if history: for child in history.added: if child is None or \ @@ -976,28 +990,22 @@ class ManyToManyDP(DependencyProcessor): tmp.update((c, state) for c in history.added + history.deleted) - if not self.passive_updates and \ - self._pks_changed(uowcommit, state): - if not history: - history = uowcommit.get_attribute_history( - state, - self.key, - passive=False) - - for child in history.unchanged: - associationrow = {} - sync.update(state, - self.parent, - associationrow, - "old_", - self.prop.synchronize_pairs) - sync.update(child, - self.mapper, - associationrow, - "old_", - self.prop.secondary_synchronize_pairs) - - secondary_update.append(associationrow) + if need_cascade_pks: + + for child in history.unchanged: + associationrow = {} + sync.update(state, + self.parent, + associationrow, + "old_", + self.prop.synchronize_pairs) + sync.update(child, + self.mapper, + associationrow, + "old_", + self.prop.secondary_synchronize_pairs) + + secondary_update.append(associationrow) if processed is not None: processed.update(tmp) diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 218d9b9cc4..d917993053 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -223,6 +223,47 @@ class RudimentaryFlushTest(UOWTest): ), ) + def test_many_to_many(self): + mapper(Item, items, properties={ + 'keywords':relationship(Keyword, secondary=item_keywords) + }) + mapper(Keyword, keywords) + + sess = create_session() + k1 = Keyword(name='k1') + i1 = Item(description='i1', keywords=[k1]) + sess.add(i1) + self.assert_sql_execution( + testing.db, + sess.flush, + AllOf( + CompiledSQL( + "INSERT INTO keywords (name) VALUES (:name)", + {'name':'k1'} + ), + CompiledSQL( + "INSERT INTO items (description) VALUES (:description)", + {'description':'i1'} + ), + ), + CompiledSQL( + "INSERT INTO item_keywords (item_id, keyword_id) " + "VALUES (:item_id, :keyword_id)", + lambda ctx:{'item_id':i1.id, 'keyword_id':k1.id} + ) + ) + + # test that keywords collection isn't loaded + sess.expire(i1, ['keywords']) + i1.description = 'i2' + self.assert_sql_execution( + testing.db, + sess.flush, + CompiledSQL("UPDATE items SET description=:description " + "WHERE items.id = :items_id", + lambda ctx:{'description':'i2', 'items_id':i1.id}) + ) + def test_m2o_flush_size(self): mapper(User, users) mapper(Address, addresses, properties={ @@ -619,12 +660,22 @@ class SingleCycleM2MTest(_base.MappedTest, (n3.id, n5.id), (n3.id, n4.id) ]) ) - + sess.delete(n1) self.assert_sql_execution( testing.db, sess.flush, + # this is n1.parents firing off, as it should, since + # passive_deletes is False for n1.parents + CompiledSQL( + "SELECT nodes.id AS nodes_id, nodes.data AS nodes_data, " + "nodes.favorite_node_id AS nodes_favorite_node_id FROM " + "nodes, node_to_nodes WHERE :param_1 = " + "node_to_nodes.right_node_id AND nodes.id = " + "node_to_nodes.left_node_id" , + lambda ctx:{u'param_1': n1.id}, + ), CompiledSQL( "DELETE FROM node_to_nodes WHERE " "node_to_nodes.left_node_id = :left_node_id AND "