From acf64c4178169b765f3f7ae492b1774955cf541f Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 6 Jun 2017 18:53:03 -0400 Subject: [PATCH] - assert passive_deletes='all' does not affect collection/scalar membership removal in issue #3844 we hypotheized that passive_deletes='all' was broken because it sets to NULL a foreign key attribute when the child object is removed or replaced. However, not doing the NULL set means that nothing happens at all and the operation silently fails. Change-Id: I11834e7e324349e172dc797bac62731008b6b95a --- lib/sqlalchemy/orm/relationships.py | 15 +++++----- test/orm/test_unitofwork.py | 45 ++++++++++++++++------------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 97adf4d8b7..94c0d66945 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -626,13 +626,14 @@ class RelationshipProperty(StrategizedProperty): database side. Additionally, setting the flag to the string value 'all' will - disable the "nulling out" of the child foreign keys, when there - is no delete or delete-orphan cascade enabled. This is - typically used when a triggering or error raise scenario is in - place on the database side. Note that the foreign key - attributes on in-session child objects will not be changed - after a flush occurs so this is a very special use-case - setting. + disable the "nulling out" of the child foreign keys, when the parent + object is deleted and there is no delete or delete-orphan cascade + enabled. This is typically used when a triggering or error raise + scenario is in place on the database side. Note that the foreign + key attributes on in-session child objects will not be changed after + a flush occurs so this is a very special use-case setting. + Additionally, the "nulling out" will still occur if the child + object is de-associated with the parent. .. seealso:: diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py index 74014781b9..09f64c1084 100644 --- a/test/orm/test_unitofwork.py +++ b/test/orm/test_unitofwork.py @@ -695,26 +695,6 @@ class ExtraPassiveDeletesTest(fixtures.MappedTest): class MyOtherClass(cls.Basic): pass - def test_assertions(self): - myothertable, MyOtherClass = (self.tables.myothertable, - self.classes.MyOtherClass) - mytable, MyClass = self.tables.mytable, self.classes.MyClass - - mapper(MyClass, mytable, properties={ - 'foo': relationship(MyOtherClass, - passive_deletes='all', - cascade="all") - }) - mapper(MyOtherClass, myothertable) - - assert_raises_message( - sa.exc.ArgumentError, - "On MyClass.foo, can't set passive_deletes='all' in conjunction " - "with 'delete' " - "or 'delete-orphan' cascade", - sa.orm.configure_mappers - ) - def test_extra_passive(self): myothertable, MyClass, MyOtherClass, mytable = ( self.tables.myothertable, @@ -770,6 +750,31 @@ class ExtraPassiveDeletesTest(fixtures.MappedTest): mc.children[0].data = 'some new data' assert_raises(sa.exc.DBAPIError, session.flush) + def test_extra_passive_obj_removed_o2m_still_nulls_out(self): + # see #3844, which we decided was not a bug + myothertable, MyClass, MyOtherClass, mytable = ( + self.tables.myothertable, + self.classes.MyClass, + self.classes.MyOtherClass, + self.tables.mytable) + + mapper(MyOtherClass, myothertable) + mapper(MyClass, mytable, properties={ + 'children': relationship(MyOtherClass, + passive_deletes='all')}) + + session = create_session() + mc = MyClass() + moc = MyOtherClass() + mc.children.append(moc) + session.add_all([mc, moc]) + session.flush() + + mc.children.remove(moc) + session.flush() + + eq_(moc.parent_id, None) + def test_dont_emit(self): myothertable, MyClass, MyOtherClass, mytable = ( self.tables.myothertable, -- 2.39.5