From 9f3b34172313382d9524941b804d1164dbc896c8 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 5 Aug 2012 22:56:13 -0400 Subject: [PATCH] - [bug] A warning is emitted when lazy='dynamic' is combined with uselist=False. This is an exception raise in 0.8. --- CHANGES | 4 +++ doc/build/orm/collections.rst | 6 ++++ lib/sqlalchemy/orm/dynamic.py | 33 +++++++++++---------- test/orm/test_dynamic.py | 54 ++++++++++++++++++++++++++++------- 4 files changed, 72 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index b0443f42d9..fd353d1521 100644 --- a/CHANGES +++ b/CHANGES @@ -18,6 +18,10 @@ CHANGES disabled on subclasses, unless overridden explicitly. + - [bug] A warning is emitted when lazy='dynamic' + is combined with uselist=False. This is an + exception raise in 0.8. + - sql - [bug] Fixed CTE bug whereby positional bound parameters present in the CTEs themselves diff --git a/doc/build/orm/collections.rst b/doc/build/orm/collections.rst index eaba3e6a85..0456cd91ad 100644 --- a/doc/build/orm/collections.rst +++ b/doc/build/orm/collections.rst @@ -84,6 +84,12 @@ Note that eager/lazy loading options cannot be used in conjunction dynamic relat The :func:`~.orm.dynamic_loader` function is essentially the same as :func:`~.orm.relationship` with the ``lazy='dynamic'`` argument specified. +.. warning:: + + The "dynamic" loader applies to **collections only**. It is not valid + to use "dynamic" loaders with many-to-one, one-to-one, or uselist=False + relationships. Newer versions of SQLAlchemy emit warnings or exceptions + in these cases. Setting Noload --------------- diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index edf0528705..b66b39f916 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -12,7 +12,6 @@ basic add/delete mutation. """ from sqlalchemy import log, util -from sqlalchemy import exc as sa_exc from sqlalchemy.orm import exc as orm_exc from sqlalchemy.sql import operators from sqlalchemy.orm import ( @@ -20,12 +19,16 @@ from sqlalchemy.orm import ( ) from sqlalchemy.orm.query import Query from sqlalchemy.orm.util import has_identity -from sqlalchemy.orm import attributes, collections +from sqlalchemy.orm import collections class DynaLoader(strategies.AbstractRelationshipLoader): def init_class_attribute(self, mapper): self.is_class_level = True - + if not self.uselist: + util.warn( + "On relationship %s, 'dynamic' loaders cannot be used with " + "many-to-one/one-to-one relationships and/or " + "uselist=False." % self.parent_property) strategies._register_attribute(self, mapper, useobject=True, @@ -63,7 +66,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl): else: return self.query_class(self, state) - def get_collection(self, state, dict_, user_data=None, + def get_collection(self, state, dict_, user_data=None, passive=attributes.PASSIVE_NO_INITIALIZE): if passive is not attributes.PASSIVE_OFF: return self._get_collection_history(state, @@ -97,7 +100,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl): if self.key not in state.committed_state: state.committed_state[self.key] = CollectionHistory(self, state) - state.modified_event(dict_, + state.modified_event(dict_, self, attributes.NEVER_SET) @@ -107,7 +110,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl): return state.committed_state[self.key] def set(self, state, dict_, value, initiator, - passive=attributes.PASSIVE_OFF, + passive=attributes.PASSIVE_OFF, check_old=None, pop=False): if initiator and initiator.parent_token is self.parent_token: return @@ -144,8 +147,8 @@ class DynamicAttributeImpl(attributes.AttributeImpl): def get_all_pending(self, state, dict_): c = self._get_collection_history(state, True) return [ - (attributes.instance_state(x), x) - for x in + (attributes.instance_state(x), x) + for x in c.added_items + c.unchanged_items + c.deleted_items ] @@ -160,12 +163,12 @@ class DynamicAttributeImpl(attributes.AttributeImpl): else: return c - def append(self, state, dict_, value, initiator, + def append(self, state, dict_, value, initiator, passive=attributes.PASSIVE_OFF): if initiator is not self: self.fire_append_event(state, dict_, value, initiator) - def remove(self, state, dict_, value, initiator, + def remove(self, state, dict_, value, initiator, passive=attributes.PASSIVE_OFF): if initiator is not self: self.fire_remove_event(state, dict_, value, initiator) @@ -204,9 +207,9 @@ class AppenderMixin(object): mapper = object_mapper(instance) prop = mapper._props[self.attr.key] self._criterion = prop.compare( - operators.eq, - instance, - value_is_parent=True, + operators.eq, + instance, + value_is_parent=True, alias_secondary=False) if self.attr.order_by: @@ -280,12 +283,12 @@ class AppenderMixin(object): def append(self, item): self.attr.append( - attributes.instance_state(self.instance), + attributes.instance_state(self.instance), attributes.instance_dict(self.instance), item, None) def remove(self, item): self.attr.remove( - attributes.instance_state(self.instance), + attributes.instance_state(self.instance), attributes.instance_dict(self.instance), item, None) diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py index ce85b20ef6..3b5d986fcf 100644 --- a/test/orm/test_dynamic.py +++ b/test/orm/test_dynamic.py @@ -1,8 +1,8 @@ from test.lib.testing import eq_, ne_ import operator -from sqlalchemy.orm import dynamic_loader, backref +from sqlalchemy.orm import dynamic_loader, backref, configure_mappers from test.lib import testing -from sqlalchemy import Integer, String, ForeignKey, desc, select, func +from sqlalchemy import Integer, String, ForeignKey, desc, select, func, exc from test.lib.schema import Table, Column from sqlalchemy.orm import mapper, relationship, create_session, Query, attributes from sqlalchemy.orm.dynamic import AppenderMixin @@ -49,12 +49,46 @@ class DynamicTest(_fixtures.FixtureTest, AssertsCompiledSQL): u = q.filter(User.id==7).first() self.assert_compile( - u.addresses.statement, + u.addresses.statement, "SELECT addresses.id, addresses.user_id, addresses.email_address FROM " "addresses WHERE :param_1 = addresses.user_id", use_default_dialect=True ) + def test_no_uselist_false(self): + users, Address, addresses, User = (self.tables.users, + self.classes.Address, + self.tables.addresses, + self.classes.User) + mapper(Address, addresses) + mapper(User, users, properties={ + "addresses": relationship(Address, lazy='dynamic', uselist=False) + }) + assert_raises_message( + exc.SAWarning, + "On relationship User.addresses, 'dynamic' loaders cannot be " + "used with many-to-one/one-to-one relationships and/or " + "uselist=False.", + configure_mappers + ) + + def test_no_m2o(self): + users, Address, addresses, User = (self.tables.users, + self.classes.Address, + self.tables.addresses, + self.classes.User) + mapper(Address, addresses, properties={ + 'user': relationship(User, lazy='dynamic') + }) + mapper(User, users) + assert_raises_message( + exc.SAWarning, + "On relationship Address.user, 'dynamic' loaders cannot be " + "used with many-to-one/one-to-one relationships and/or " + "uselist=False.", + configure_mappers + ) + def test_order_by(self): users, Address, addresses, User = (self.tables.users, self.classes.Address, @@ -62,13 +96,13 @@ class DynamicTest(_fixtures.FixtureTest, AssertsCompiledSQL): self.classes.User) mapper(User, users, properties={ - 'addresses':dynamic_loader(mapper(Address, addresses)) + 'addresses': dynamic_loader(mapper(Address, addresses)) }) sess = create_session() u = sess.query(User).get(8) eq_( list(u.addresses.order_by(desc(Address.email_address))), - [Address(email_address=u'ed@wood.com'), Address(email_address=u'ed@lala.com'), + [Address(email_address=u'ed@wood.com'), Address(email_address=u'ed@lala.com'), Address(email_address=u'ed@bettyboop.com')] ) @@ -191,7 +225,7 @@ class DynamicTest(_fixtures.FixtureTest, AssertsCompiledSQL): assert o1 in i1.orders.all() assert i1 in o1.items.all() - @testing.exclude('mysql', 'between', + @testing.exclude('mysql', 'between', ((5, 1,49), (5, 1, 52)), 'https://bugs.launchpad.net/ubuntu/+source/mysql-5.1/+bug/706988') def test_association_nonaliased(self): @@ -202,8 +236,8 @@ class DynamicTest(_fixtures.FixtureTest, AssertsCompiledSQL): self.classes.Item) mapper(Order, orders, properties={ - 'items':relationship(Item, secondary=order_items, - lazy="dynamic", + 'items':relationship(Item, secondary=order_items, + lazy="dynamic", order_by=order_items.c.item_id) }) mapper(Item, items) @@ -221,7 +255,7 @@ class DynamicTest(_fixtures.FixtureTest, AssertsCompiledSQL): use_default_dialect=True ) - # filter criterion against the secondary table + # filter criterion against the secondary table # works eq_( o.items.filter(order_items.c.item_id==2).all(), @@ -488,7 +522,7 @@ class SessionTest(_fixtures.FixtureTest): sess.flush() sess.commit() u1.addresses.append(Address(email_address='foo@bar.com')) - eq_(u1.addresses.order_by(Address.id).all(), + eq_(u1.addresses.order_by(Address.id).all(), [Address(email_address='lala@hoho.com'), Address(email_address='foo@bar.com')]) sess.rollback() eq_(u1.addresses.all(), [Address(email_address='lala@hoho.com')]) -- 2.47.2