From: Mike Bayer Date: Thu, 15 Jan 2009 18:08:48 +0000 (+0000) Subject: - Using delete-orphan on a many-to-many relation is deprecated. X-Git-Tag: rel_0_5_1~10 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=79a7f1723ab86d560a98aa4a10f1861236e6fde1;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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] --- diff --git a/CHANGES b/CHANGES index ecb9fa3865..ac92141434 100644 --- 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 diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index a4561d443d..7198cc830f 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -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: diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 0288f9964a..f4ba49ae1e 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -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) diff --git a/test/orm/cascade.py b/test/orm/cascade.py index 0a3f800d3f..3bf7913e49 100644 --- a/test/orm/cascade.py +++ b/test/orm/cascade.py @@ -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""" diff --git a/test/orm/unitofwork.py b/test/orm/unitofwork.py index 553713da53..bb84bfd5bd 100644 --- a/test/orm/unitofwork.py +++ b/test/orm/unitofwork.py @@ -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)