From: Mike Bayer Date: Sun, 12 Sep 2010 16:37:43 +0000 (-0400) Subject: - Slight improvement to the behavior of "passive_updates=False" X-Git-Tag: rel_0_6_5~76 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bad4493842caeba312403efb143144a91e562311;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Slight improvement to the behavior of "passive_updates=False" when placed only on the many-to-one side of a relationship; documentation has been clarified that passive_updates=False should really be on the one-to-many side. - Placing passive_deletes=True on a many-to-one emits a warning, since you probably intended to put it on the one-to-many side. --- diff --git a/CHANGES b/CHANGES index 6d783757de..3f84d2b896 100644 --- a/CHANGES +++ b/CHANGES @@ -11,6 +11,16 @@ CHANGES on "newly persistent" objects. This can occur when user defined code inadvertently triggers flushes on not-fully-loaded objects. + + - Slight improvement to the behavior of "passive_updates=False" + when placed only on the many-to-one side of a + relationship; documentation has been clarified + that passive_updates=False should really be on the + one-to-many side. + + - Placing passive_deletes=True on a many-to-one emits + a warning, since you probably intended to put it on + the one-to-many side. 0.6.4 ===== diff --git a/doc/build/orm/relationships.rst b/doc/build/orm/relationships.rst index 0392a68e6a..3428473288 100644 --- a/doc/build/orm/relationships.rst +++ b/doc/build/orm/relationships.rst @@ -719,15 +719,26 @@ row at a time for the time being). Mutable Primary Keys / Update Cascades --------------------------------------- -When the primary key of an entity changes, related items which reference the -primary key must also be updated as well. For databases which enforce -referential integrity, it's required to use the database's ON UPDATE CASCADE -functionality in order to propagate primary key changes. For those which -don't, the ``passive_updates`` flag can be set to ``False`` which instructs -SQLAlchemy to issue UPDATE statements individually. The ``passive_updates`` -flag can also be ``False`` in conjunction with ON UPDATE CASCADE -functionality, although in that case it issues UPDATE statements -unnecessarily. +When the primary key of an entity changes, related items +which reference the primary key must also be updated as +well. For databases which enforce referential integrity, +it's required to use the database's ON UPDATE CASCADE +functionality in order to propagate primary key changes +to referenced foreign keys - the values cannot be out +of sync for any moment. + +For databases that don't support this, such as SQLite and +MySQL without their referential integrity options turned +on, the ``passive_updates`` flag can +be set to ``False``, most preferably on a one-to-many or +many-to-many :func:`.relationship`, which instructs +SQLAlchemy to issue UPDATE statements individually for +objects referenced in the collection, loading them into +memory if not already locally present. The +``passive_updates`` flag can also be ``False`` in +conjunction with ON UPDATE CASCADE functionality, +although in that case the unit of work will be issuing +extra SELECT and UPDATE statements unnecessarily. A typical mutable primary key setup might look like: @@ -746,12 +757,28 @@ A typical mutable primary key setup might look like: class Address(object): pass + # passive_updates=False *only* needed if the database + # does not implement ON UPDATE CASCADE + mapper(User, users, properties={ 'addresses': relationship(Address, passive_updates=False) }) mapper(Address, addresses) -passive_updates is set to ``True`` by default. Foreign key references to non-primary key columns are supported as well. +``passive_updates`` is set to ``True`` by default, +indicating that ON UPDATE CASCADE is expected to be in +place in the usual case for foreign keys that expect +to have a mutating parent key. + +``passive_updates=False`` may be configured on any +direction of relationship, i.e. one-to-many, many-to-one, +and many-to-many, although it is much more effective when +placed just on the one-to-many or many-to-many side. +Configuring the ``passive_updates=False`` only on the +many-to-one side will have only a partial effect, as the +unit of work searches only through the current identity +map for objects that may be referencing the one with a +mutating primary key, not throughout the database. The :func:`relationship` API ---------------------------- diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 376afd88d4..662cfc67b0 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -734,7 +734,12 @@ class DetectKeySwitch(DependencyProcessor): def per_property_preprocessors(self, uow): if self.prop._reverse_property: - return + if self.passive_updates: + return + else: + if False in (prop.passive_updates for \ + prop in self.prop._reverse_property): + return uow.register_preprocessor(self, False) @@ -797,14 +802,12 @@ class DetectKeySwitch(DependencyProcessor): if switchers: # if primary key values have actually changed somewhere, perform # a linear search through the UOW in search of a parent. - # note that this handler isn't used if the many-to-one - # relationship has a backref. for state in uowcommit.session.identity_map.all_states(): if not issubclass(state.class_, self.parent.class_): continue dict_ = state.dict - related = dict_.get(self.key) - if related is not None: + related = state.get_impl(self.key).get(state, dict_, passive=self.passive_updates) + if related is not attributes.PASSIVE_NO_RESULT and related is not None: related_state = attributes.instance_state(dict_[self.key]) if related_state in switchers: uowcommit.register_object(state, diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 1b8a4b5455..6098339d25 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -1215,6 +1215,10 @@ class RelationshipProperty(StrategizedProperty): 'when single_parent is not set. Set ' 'single_parent=True on the relationship().' % self) + if self.direction is MANYTOONE and self.passive_deletes: + util.warn("On %s, 'passive_deletes' is normally configured " + "on one-to-many, one-to-one, many-to-many relationships only." + % self) def _determine_local_remote_pairs(self): if not self.local_remote_pairs: diff --git a/test/orm/test_naturalpks.py b/test/orm/test_naturalpks.py index b305375da1..d02ecb7075 100644 --- a/test/orm/test_naturalpks.py +++ b/test/orm/test_naturalpks.py @@ -8,7 +8,7 @@ import sqlalchemy as sa from sqlalchemy.test import testing from sqlalchemy import Integer, String, ForeignKey, Unicode from sqlalchemy.test.schema import Table, Column -from sqlalchemy.orm import mapper, relationship, create_session, backref +from sqlalchemy.orm import mapper, relationship, create_session, backref, Session from sqlalchemy.orm.session import make_transient from sqlalchemy.test.testing import eq_ from test.orm import _base, _fixtures @@ -499,24 +499,49 @@ class SelfReferentialTest(_base.MappedTest): pass @testing.resolve_artifact_names - def test_one_to_many(self): + def test_one_to_many_on_m2o(self): mapper(Node, nodes, properties={ 'children': relationship(Node, backref=sa.orm.backref('parentnode', remote_side=nodes.c.name, passive_updates=False), - passive_updates=False)}) + )}) - sess = create_session() + sess = Session() + n1 = Node(name='n1') + sess.add(n1) + n2 = Node(name='n11', parentnode=n1) + n3 = Node(name='n12', parentnode=n1) + n4 = Node(name='n13', parentnode=n1) + sess.add_all([n2, n3, n4]) + sess.commit() + + n1.name = 'new n1' + sess.commit() + eq_(['new n1', 'new n1', 'new n1'], + [n.parent + for n in sess.query(Node).filter( + Node.name.in_(['n11', 'n12', 'n13']))]) + + @testing.resolve_artifact_names + def test_one_to_many_on_o2m(self): + mapper(Node, nodes, properties={ + 'children': relationship(Node, + backref=sa.orm.backref('parentnode', + remote_side=nodes.c.name), + passive_updates=False + )}) + + sess = Session() n1 = Node(name='n1') n1.children.append(Node(name='n11')) n1.children.append(Node(name='n12')) n1.children.append(Node(name='n13')) sess.add(n1) - sess.flush() + sess.commit() n1.name = 'new n1' - sess.flush() + sess.commit() eq_(n1.children[1].parent, 'new n1') eq_(['new n1', 'new n1', 'new n1'], [n.parent @@ -540,18 +565,16 @@ class SelfReferentialTest(_base.MappedTest): } ) - sess = create_session() + sess = Session() n1 = Node(name='n1') n11 = Node(name='n11', parentnode=n1) n12 = Node(name='n12', parentnode=n1) n13 = Node(name='n13', parentnode=n1) sess.add_all([n1, n11, n12, n13]) - sess.flush() + sess.commit() n1.name = 'new n1' - sess.flush() - if passive: - sess.expire_all() + sess.commit() eq_(['new n1', 'new n1', 'new n1'], [n.parent for n in sess.query(Node).filter( diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py index da0f2a9e4b..c95836055b 100644 --- a/test/orm/test_unitofwork.py +++ b/test/orm/test_unitofwork.py @@ -699,10 +699,15 @@ class PassiveDeletesTest(_base.MappedTest): assert mytable.count().scalar() == 0 assert myothertable.count().scalar() == 0 + @testing.emits_warning(r".*'passive_deletes' is normally configured on one-to-many") @testing.resolve_artifact_names def test_backwards_pd(self): - # the unusual scenario where a trigger or something might be deleting - # a many-to-one on deletion of the parent row + """Test that passive_deletes=True disables a delete from an m2o. + + This is not the usual usage and it now raises a warning, but test + that it works nonetheless. + + """ mapper(MyOtherClass, myothertable, properties={ 'myclass':relationship(MyClass, cascade="all, delete", passive_deletes=True) }) @@ -722,8 +727,17 @@ class PassiveDeletesTest(_base.MappedTest): session.delete(mco) session.flush() + # mytable wasn't deleted, is the point. assert mytable.count().scalar() == 1 assert myothertable.count().scalar() == 0 + + @testing.resolve_artifact_names + def test_aaa_m2o_emits_warning(self): + mapper(MyOtherClass, myothertable, properties={ + 'myclass':relationship(MyClass, cascade="all, delete", passive_deletes=True) + }) + mapper(MyClass, mytable) + assert_raises(sa.exc.SAWarning, sa.orm.compile_mappers) class ExtraPassiveDeletesTest(_base.MappedTest): __requires__ = ('foreign_keys',)