From: Mike Bayer Date: Sun, 28 Nov 2010 18:01:23 +0000 (-0500) Subject: - cascade_backrefs flag on relationship() now set to False by default. X-Git-Tag: rel_0_7b1~230 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e836366c843cd64a0df569582534868e3fb00f3b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - cascade_backrefs flag on relationship() now set to False by default. [ticket:1974] --- diff --git a/doc/build/orm/session.rst b/doc/build/orm/session.rst index 061f16f7ed..d40ada7b2b 100644 --- a/doc/build/orm/session.rst +++ b/doc/build/orm/session.rst @@ -428,7 +428,7 @@ Lets use the canonical example of the User and Address objects:: id = Column(Integer, primary_key=True) name = Column(String(50), nullable=False) - addresses = relationship("Address", backref="user") + addresses = relationship("Address", backref="user", cascade_backrefs=True) class Address(Base): __tablename__ = 'address' @@ -472,21 +472,17 @@ and made our ``a1`` object pending, as though we had added it. Now we have >>> a1 is existing_a1 False -Above, our ``a1`` is already pending in the session. The -subsequent :meth:`~.Session.merge` operation essentially -does nothing. Cascade can be configured via the ``cascade`` -option on :func:`.relationship`, although in this case it -would mean removing the ``save-update`` cascade from the -``User.addresses`` relationship - and usually, that behavior -is extremely convenient. The solution here would usually be to not assign -``a1.user`` to an object already persistent in the target -session. +Above, our ``a1`` is already pending in the session. The subsequent +:meth:`~.Session.merge` operation essentially does nothing. To resolve this +issue, the ``cascade_backrefs`` flag should be set to its default of +``False``. Further detail on cascade operation is at +:ref:`unitofwork_cascades`. + -Note that a new :func:`.relationship` option introduced in 0.6.5, -``cascade_backrefs=False``, will also prevent the ``Address`` from -being added to the session via the ``a1.user = u1`` assignment. +.. note:: Up until version 0.7 of SQLAlchemy, the "cascade backrefs" flag + defaulted to ``True`` and prior to 0.6.5 there was no way to disable it, + without turning off "save-update" cascade entirely. -Further detail on cascade operation is at :ref:`unitofwork_cascades`. Another example of unexpected state:: @@ -884,8 +880,8 @@ objects to allow attachment to only one parent at a time. The default value for ``cascade`` on :func:`~sqlalchemy.orm.relationship` is ``save-update, merge``. -``save-update`` cascade also takes place on backrefs by default. This means -that, given a mapping such as this:: +``save-update`` cascade, by default, does not take place on backrefs (new in 0.7). +This means that, given a mapping such as this:: mapper(Order, order_table, properties={ 'items' : relationship(Item, items_table, backref='order') @@ -893,8 +889,8 @@ that, given a mapping such as this:: If an ``Order`` is already in the session, and is assigned to the ``order`` attribute of an ``Item``, the backref appends the ``Item`` to the ``orders`` -collection of that ``Order``, resulting in the ``save-update`` cascade taking -place:: +collection of that ``Order``, however, the ``Item`` will not yet be present +in the session:: >>> o1 = Order() >>> session.add(o1) @@ -906,22 +902,34 @@ place:: >>> i1 in o1.orders True >>> i1 in session - True - -This behavior can be disabled as of 0.6.5 using the ``cascade_backrefs`` flag:: + False + +You can of course :func:`~.Session.add` ``i1`` to the session at a later +point. SQLAlchemy defaults to this behavior as of 0.7 as it is helpful for +situations where an object needs to be kept out of a session until it's +construction is completed, but still needs to be given associations to objects +which are already persistent in the target session. It's more intuitive +that "cascades" into a :class:`.Session` work from left to right only, and also +allows session membership behavior to be more compatible with relationships that +don't have backrefs configured. + +The save-update cascade can be made bi-directional using ``cascade_backrefs`` flag:: mapper(Order, order_table, properties={ 'items' : relationship(Item, items_table, backref='order', - cascade_backrefs=False) + cascade_backrefs=True) }) -So above, the assignment of ``i1.order = o1`` will append ``i1`` to the ``orders`` -collection of ``o1``, but will not add ``i1`` to the session. You can of -course :func:`~.Session.add` ``i1`` to the session at a later point. This option -may be helpful for situations where an object needs to be kept out of a -session until it's construction is completed, but still needs to be given -associations to objects which are already persistent in the target session. +Using the above mapping with the previous usage example, +the assignment of ``i1.order = o1`` will append ``i1`` to the ``orders`` +collection of ``o1``, and will add ``i1`` to the session. ``cascade_backrefs`` +is specific to just one direction, so the above configuration still would not +cascade an ``Order`` into the session if it were associated with an ``Item`` +already in the session, unless ``cascade_backrefs`` were also configured on the +"order" side of the relationship. +Setting ``cascade_backrefs=True`` on a relationship and its backref makes the +operation compatible with the default behavior of SQLAlchemy 0.6 and earlier. .. _unitofwork_transaction: diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 659484691e..d2f5de2b92 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -267,17 +267,18 @@ def relationship(argument, secondary=None, **kwargs): * ``all`` - shorthand for "save-update,merge, refresh-expire, expunge, delete" - :param cascade_backrefs=True: + :param cascade_backrefs=False: a boolean value indicating if the ``save-update`` cascade should - operate along a backref event. When set to ``False`` on a + operate along a backref event. When set to ``True`` on a one-to-many relationship that has a many-to-one backref, assigning a persistent object to the many-to-one attribute on a transient object - will not add the transient to the session. Similarly, when - set to ``False`` on a many-to-one relationship that has a one-to-many + will add the transient to the session. Similarly, when + set to ``True`` on a many-to-one relationship that has a one-to-many backref, appending a persistent object to the one-to-many collection - on a transient object will not add the transient to the session. + on a transient object will add the transient to the session. - ``cascade_backrefs`` is new in 0.6.5. + ``cascade_backrefs`` is new in 0.6.5. The default value changes + from ``True`` to ``False`` as of the 0.7 series. :param collection_class: a class or callable that returns a new list-holding object. will diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index b68290dbdd..664eba62f5 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -452,7 +452,7 @@ class RelationshipProperty(StrategizedProperty): single_parent=False, innerjoin=False, doc=None, active_history=False, - cascade_backrefs=True, + cascade_backrefs=False, load_on_pending=False, strategy_class=None, _local_remote_pairs=None, query_class=None): diff --git a/test/orm/inheritance/test_polymorph.py b/test/orm/inheritance/test_polymorph.py index 7296bec98e..33646c9227 100644 --- a/test/orm/inheritance/test_polymorph.py +++ b/test/orm/inheritance/test_polymorph.py @@ -258,6 +258,7 @@ def _generate_round_trip_test(include_base, lazy_relationship, redefine_colprop, c = session.query(Company).first() daboss.company = c + session.add(daboss) manager_list = [e for e in c.employees if isinstance(e, Manager)] session.flush() session.expunge_all() diff --git a/test/orm/test_assorted_eager.py b/test/orm/test_assorted_eager.py index 0e389b74ba..ff9234d701 100644 --- a/test/orm/test_assorted_eager.py +++ b/test/orm/test_assorted_eager.py @@ -2,6 +2,9 @@ Derived from mailing list-reported problems and trac tickets. +These are generally very old 0.1-era tests and at some point should +be cleaned up and modernized. + """ import datetime @@ -580,12 +583,6 @@ class EagerTest7(_base.MappedTest): Column('company_id', Integer, ForeignKey("companies.company_id")), Column('date', sa.DateTime)) - Table('items', metadata, - Column('item_id', Integer, primary_key=True, test_needs_autoincrement=True), - Column('invoice_id', Integer, ForeignKey('invoices.invoice_id')), - Column('code', String(20)), - Column('qty', Integer)) - @classmethod def setup_classes(cls): class Company(_base.ComparableEntity): @@ -597,14 +594,11 @@ class EagerTest7(_base.MappedTest): class Phone(_base.ComparableEntity): pass - class Item(_base.ComparableEntity): - pass - class Invoice(_base.ComparableEntity): pass @testing.resolve_artifact_names - def testone(self): + def test_load_m2o_attached_to_o2m(self): """ Tests eager load of a many-to-one attached to a one-to-many. this testcase illustrated the bug, which is that when the single Company is @@ -638,66 +632,12 @@ class EagerTest7(_base.MappedTest): session.expunge_all() i = session.query(Invoice).get(invoice_id) + + def go(): + eq_(c, i.company) + eq_(c.addresses, i.company.addresses) + self.assert_sql_count(testing.db, go, 0) - eq_(c, i.company) - - @testing.resolve_artifact_names - def testtwo(self): - """The original testcase that includes various complicating factors""" - - mapper(Phone, phone_numbers) - - mapper(Address, addresses, properties={ - 'phones': relationship(Phone, lazy='joined', backref='address', - order_by=phone_numbers.c.phone_id)}) - - mapper(Company, companies, properties={ - 'addresses': relationship(Address, lazy='joined', backref='company', - order_by=addresses.c.address_id)}) - - mapper(Item, items) - - mapper(Invoice, invoices, properties={ - 'items': relationship(Item, lazy='joined', backref='invoice', - order_by=items.c.item_id), - 'company': relationship(Company, lazy='joined', backref='invoices')}) - - c1 = Company(company_name='company 1', addresses=[ - Address(address='a1 address', - phones=[Phone(type='home', number='1111'), - Phone(type='work', number='22222')]), - Address(address='a2 address', - phones=[Phone(type='home', number='3333'), - Phone(type='work', number='44444')]) - ]) - - session = create_session() - session.add(c1) - session.flush() - - company_id = c1.company_id - - session.expunge_all() - - a = session.query(Company).get(company_id) - - # set up an invoice - i1 = Invoice(date=datetime.datetime.now(), company=a) - - item1 = Item(code='aaaa', qty=1, invoice=i1) - item2 = Item(code='bbbb', qty=2, invoice=i1) - item3 = Item(code='cccc', qty=3, invoice=i1) - - session.flush() - invoice_id = i1.invoice_id - - session.expunge_all() - c = session.query(Company).get(company_id) - - session.expunge_all() - i = session.query(Invoice).get(invoice_id) - - eq_(c, i.company) class EagerTest8(_base.MappedTest): diff --git a/test/orm/test_backref_mutations.py b/test/orm/test_backref_mutations.py index 42b6054b37..dd48df592d 100644 --- a/test/orm/test_backref_mutations.py +++ b/test/orm/test_backref_mutations.py @@ -438,6 +438,7 @@ class O2OScalarOrphanTest(_fixtures.FixtureTest): # the _SingleParent extension sets the backref get to "active" ! # u1 gets loaded and deleted u2.address = a1 + sess.add(u2) sess.commit() assert sess.query(User).count() == 1 diff --git a/test/orm/test_cascade.py b/test/orm/test_cascade.py index 0deef18fd1..bedff21704 100644 --- a/test/orm/test_cascade.py +++ b/test/orm/test_cascade.py @@ -1472,13 +1472,15 @@ class NoBackrefCascadeTest(_fixtures.FixtureTest): def setup_mappers(cls): mapper(Address, addresses) mapper(User, users, properties={ - 'addresses':relationship(Address, backref='user', - cascade_backrefs=False) + 'addresses':relationship(Address, + backref=backref('user', cascade_backrefs=True), + ) }) mapper(Dingaling, dingalings, properties={ - 'address' : relationship(Address, backref='dingalings', - cascade_backrefs=False) + 'address' : relationship(Address, + backref=backref('dingalings', cascade_backrefs=True), + ) }) @testing.resolve_artifact_names diff --git a/test/orm/test_expire.py b/test/orm/test_expire.py index c8bdf1719e..0d9f5e7453 100644 --- a/test/orm/test_expire.py +++ b/test/orm/test_expire.py @@ -973,6 +973,7 @@ class ExpiredPendingTest(_fixtures.FixtureTest): u1 = User(name='u1') a1.user = u1 + sess.flush() # expire 'addresses'. backrefs @@ -983,7 +984,8 @@ class ExpiredPendingTest(_fixtures.FixtureTest): # in user.addresses a2 = Address(email_address='a2') a2.user = u1 - + sess.add(a2) + # expire u1.addresses again. this expires # "pending" as well. sess.expire(u1, ['addresses']) @@ -994,6 +996,7 @@ class ExpiredPendingTest(_fixtures.FixtureTest): # only two addresses pulled from the DB, no "pending" assert len(u1.addresses) == 2 + # flushes a2 sess.flush() sess.expire_all() assert len(u1.addresses) == 3 diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index 4438ab8f70..78e0f22063 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -3068,7 +3068,7 @@ class RequirementsTest(_base.MappedTest): h6 = H6() h6.h1a = h1 h6.h1b = x = H1() - assert x in s + s.add(x) h6.h1b.h2s.append(H2()) diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py index 15c52a1e01..50d001a8c2 100644 --- a/test/orm/test_merge.py +++ b/test/orm/test_merge.py @@ -4,8 +4,9 @@ from sqlalchemy import Integer, PickleType, String import operator from sqlalchemy.test import testing from sqlalchemy.util import OrderedSet -from sqlalchemy.orm import mapper, relationship, create_session, PropComparator, \ - synonym, comparable_property, sessionmaker, attributes +from sqlalchemy.orm import mapper, relationship, create_session, \ + PropComparator, synonym, comparable_property, sessionmaker, \ + attributes, Session from sqlalchemy.orm.collections import attribute_mapped_collection from sqlalchemy.orm.interfaces import MapperOption from sqlalchemy.test.testing import eq_, ne_ @@ -876,13 +877,22 @@ class MergeTest(_fixtures.FixtureTest): def test_cascade_doesnt_blowaway_manytoone(self): """a merge test that was fixed by [ticket:1202]""" - s = create_session(autoflush=True) + s = Session() + + mapper(Address, addresses) mapper(User, users, properties={ - 'addresses':relationship(mapper(Address, addresses),backref='user')}) + 'addresses':relationship(Address,backref='user') + }) - a1 = Address(user=s.merge(User(id=1, name='ed')), email_address='x') + u1 = s.merge(User(id=1, name='ed')) + a1 = Address(user=u1, email_address='x') + s.add(a1) + before_id = id(a1.user) - a2 = Address(user=s.merge(User(id=1, name='jack')), email_address='x') + + # autoflushes a1, u1 + u2 = s.merge(User(id=1, name='jack')) + a2 = Address(user=u2, email_address='x') after_id = id(a1.user) other_id = id(a2.user) eq_(before_id, other_id)