]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
- Using delete-orphan on a many-to-many relation is deprecated.
authorMike Bayer <mike_mp@zzzcomputing.com>
Thu, 15 Jan 2009 18:08:48 +0000 (18:08 +0000)
committerMike Bayer <mike_mp@zzzcomputing.com>
Thu, 15 Jan 2009 18:08:48 +0000 (18:08 +0000)
This produces misleading or erroneous results since SQLA does
not retrieve the full list of "parents" for m2m.  To get delete-orphan
behavior with an m2m table, use an explcit association class
so that the individual association row is treated as a parent.
[ticket:1281]

- delete-orphan cascade always requires delete cascade.  Specifying
delete-orphan without delete now raises a deprecation warning.
[ticket:1281]

CHANGES
lib/sqlalchemy/orm/properties.py
lib/sqlalchemy/orm/util.py
test/orm/cascade.py
test/orm/unitofwork.py

diff --git a/CHANGES b/CHANGES
index ecb9fa3865a9702c2ddf7b368f569bf5e583d36d..ac9214143463f2a2fefa7acae7e65cec484bcddd 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -60,6 +60,17 @@ CHANGES
       which contained no defined values for the child table where
       an UPDATE with no SET clause would be rendered.
 
+    - Using delete-orphan on a many-to-many relation is deprecated.
+      This produces misleading or erroneous results since SQLA does 
+      not retrieve the full list of "parents" for m2m.  To get delete-orphan
+      behavior with an m2m table, use an explcit association class
+      so that the individual association row is treated as a parent.
+      [ticket:1281]
+    
+    - delete-orphan cascade always requires delete cascade.  Specifying
+      delete-orphan without delete now raises a deprecation warning.
+      [ticket:1281]
+      
 - sql
     - Improved the methodology to handling percent signs in column
       names from [ticket:1256].  Added more tests.  MySQL and
index a4561d443df6300764f107c9cd3b5d31149252c6..7198cc830fe0e2db36fc7a9de0891f8e978a86f9 100644 (file)
@@ -910,7 +910,11 @@ class RelationProperty(StrategizedProperty):
                     "- foreign key columns are present in both the parent and "
                     "the child's mapped tables.  Specify 'foreign_keys' "
                     "argument." % (str(self)))
-
+        
+        if self.cascade.delete_orphan and self.direction is MANYTOMANY:
+            util.warn("On %s, delete-orphan cascade is not supported on a "
+                    "many-to-many relation.  This will raise an error in 0.6." % self)
+        
     def _determine_local_remote_pairs(self):
         if not self.local_remote_pairs:
             if self.remote_side:
index 0288f9964ac36ee8e0b63221e3a0fdbd0631eb46..f4ba49ae1ec0443695ef794bd3c3c219ecef2a69 100644 (file)
@@ -31,6 +31,11 @@ class CascadeOptions(object):
         self.merge = "merge" in values or "all" in values
         self.expunge = "expunge" in values or "all" in values
         self.refresh_expire = "refresh-expire" in values or "all" in values
+        
+        if self.delete_orphan and not self.delete:
+            util.warn("The 'delete-orphan' cascade option requires "
+                        "'delete'.  This will raise an error in 0.6.")
+            
         for x in values:
             if x not in all_cascades:
                 raise sa_exc.ArgumentError("Invalid cascade option '%s'" % x)
index 0a3f800d3ff0b1860f9b40c937b2363c9803e64b..3bf7913e4908c4d8f58ccbdb9babdc62398d4eea 100644 (file)
@@ -626,20 +626,33 @@ class M2OCascadeDeleteOrphanTest(_base.MappedTest):
         eq_(sess.query(T3).all(), [])
 
 class M2MCascadeTest(_base.MappedTest):
+    """delete-orphan cascade is deprecated on many-to-many."""
+    
     def define_tables(self, metadata):
         Table('a', metadata,
             Column('id', Integer, primary_key=True),
-            Column('data', String(30)))
+            Column('data', String(30)),
+            test_needs_fk=True
+            )
         Table('b', metadata,
             Column('id', Integer, primary_key=True),
-            Column('data', String(30)))
+            Column('data', String(30)),
+            test_needs_fk=True
+            
+            )
         Table('atob', metadata,
             Column('aid', Integer, ForeignKey('a.id')),
-            Column('bid', Integer, ForeignKey('b.id')))
+            Column('bid', Integer, ForeignKey('b.id')),
+            test_needs_fk=True
+            
+            )
         Table('c', metadata,
               Column('id', Integer, primary_key=True),
               Column('data', String(30)),
-              Column('bid', Integer, ForeignKey('b.id')))
+              Column('bid', Integer, ForeignKey('b.id')),
+              test_needs_fk=True
+              
+              )
 
     def setup_classes(self):
         class A(_fixtures.Base):
@@ -649,6 +662,7 @@ class M2MCascadeTest(_base.MappedTest):
         class C(_fixtures.Base):
             pass
 
+    @testing.emits_warning(".*not supported on a many-to-many")
     @testing.resolve_artifact_names
     def test_delete_orphan(self):
         mapper(A, a, properties={
@@ -670,6 +684,7 @@ class M2MCascadeTest(_base.MappedTest):
         assert b.count().scalar() == 0
         assert a.count().scalar() == 1
 
+    @testing.emits_warning(".*not supported on a many-to-many")
     @testing.resolve_artifact_names
     def test_delete_orphan_cascades(self):
         mapper(A, a, properties={
@@ -693,6 +708,7 @@ class M2MCascadeTest(_base.MappedTest):
         assert a.count().scalar() == 1
         assert c.count().scalar() == 0
 
+    @testing.emits_warning(".*not supported on a many-to-many")
     @testing.resolve_artifact_names
     def test_cascade_delete(self):
         mapper(A, a, properties={
@@ -711,6 +727,39 @@ class M2MCascadeTest(_base.MappedTest):
         assert b.count().scalar() == 0
         assert a.count().scalar() == 0
 
+    @testing.emits_warning(".*not supported on a many-to-many")
+    @testing.fails_on_everything_except('sqlite')
+    @testing.resolve_artifact_names
+    def test_this_doesnt_work(self):
+        """illustrates why cascade with m2m should not be supported
+            (i.e. many parents...)
+            
+        """
+        mapper(A, a, properties={
+            'bs':relation(B, secondary=atob, cascade="all, delete-orphan")
+        })
+        mapper(B, b)
+
+        sess = create_session()
+        b1 =B(data='b1')
+        a1 = A(data='a1', bs=[b1])
+        a2 = A(data='a2', bs=[b1])
+        sess.add(a1)
+        sess.add(a2)
+        sess.flush()
+
+        sess.delete(a1)
+        
+        # this raises an integrity error on DBs that support FKs
+        sess.flush()
+        
+        # still a row present !
+        assert atob.count().scalar() ==1
+        
+        # but no bs !
+        assert b.count().scalar() == 0
+        assert a.count().scalar() == 1
+
 
 class UnsavedOrphansTest(_base.MappedTest):
     """Pending entities that are orphans"""
index 553713da539fe233bcbe1d1f2d9d793463d9e331..bb84bfd5bde6741b09e273d10e15afb80105caca 100644 (file)
@@ -2160,7 +2160,7 @@ class RowSwitchTest(_base.MappedTest):
     @testing.resolve_artifact_names
     def test_manytomany(self):
         mapper(T5, t5, properties={
-            't7s':relation(T7, secondary=t5t7, cascade="all, delete-orphan")
+            't7s':relation(T7, secondary=t5t7, cascade="all")
         })
         mapper(T7, t7)