From: Mike Bayer Date: Fri, 14 Oct 2016 21:06:07 +0000 (-0400) Subject: Assemble "don't joinedload other side" rule using query._current_path X-Git-Tag: rel_1_1_2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ae7d2837b3c5ae3fd6e9dad6b14a26abb32cfee5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Assemble "don't joinedload other side" rule using query._current_path Discovered during testing for [ticket:3822], the rule added for [ticket:1495] will fail if the source object has propagated options set up, which add elements to query._current_path. Fixes: #3824 Change-Id: I3d96c96fee5f9b247f739d2136d18681ac61f2fe --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index b90b932b55..127227ded9 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -31,6 +31,14 @@ was a ``functools.partial`` or other object that doesn't have a ``__module__`` attribute. + .. change:: + :tags: bug, orm + :tickets: 3824 + + Fixed bug involving the rule to disable a joined collection eager + loader on the other side of a many-to-one lazy loader, first added + in [ticket:1495], where the rule would fail if the parent object + had some other lazyloader-bound query options associated with it. .. changelog:: :version: 1.1.1 diff --git a/lib/sqlalchemy/ext/baked.py b/lib/sqlalchemy/ext/baked.py index 3ca94925eb..2f658edf3a 100644 --- a/lib/sqlalchemy/ext/baked.py +++ b/lib/sqlalchemy/ext/baked.py @@ -467,11 +467,15 @@ class BakedLazyLoader(strategies.LazyLoader): if rev.direction is interfaces.MANYTOONE and \ rev._use_get and \ not isinstance(rev.strategy, strategies.LazyLoader): + q.add_criteria( lambda q: q.options( - strategy_options.Load( - rev.parent).baked_lazyload(rev.key))) + strategy_options.Load.for_existing_path( + q._current_path[rev.parent] + ).baked_lazyload(rev.key) + ) + ) lazy_clause, params = self._generate_lazy_clause(state, passive) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index b2cd5b5ec8..5f5ab10690 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -615,7 +615,10 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): rev._use_get and \ not isinstance(rev.strategy, LazyLoader): q = q.options( - strategy_options.Load(rev.parent).lazyload(rev.key)) + strategy_options.Load.for_existing_path( + q._current_path[rev.parent] + ).lazyload(rev.key) + ) lazy_clause, params = self._generate_lazy_clause( state, passive=passive) diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index 2fb13f3cfa..0c1ebf404e 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -85,6 +85,14 @@ class Load(Generative, MapperOption): self.context = {} self.local_opts = {} + @classmethod + def for_existing_path(cls, path): + load = cls.__new__(cls) + load.path = path + load.context = {} + load.local_opts = {} + return load + def _generate(self): cloned = super(Load, self)._generate() cloned.local_opts = {} diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py index 2f501eb6c0..818019de2b 100644 --- a/test/ext/test_baked.py +++ b/test/ext/test_baked.py @@ -1,5 +1,5 @@ from sqlalchemy.orm import Session, subqueryload, \ - mapper, relationship, lazyload, clear_mappers + mapper, relationship, lazyload, clear_mappers, backref from sqlalchemy.testing import eq_, is_, is_not_ from sqlalchemy.testing import assert_raises, assert_raises_message from sqlalchemy import testing @@ -628,7 +628,9 @@ class ResultTest(BakedTest): sess.close() -class LazyLoaderTest(BakedTest): +from sqlalchemy.testing.assertsql import CompiledSQL + +class LazyLoaderTest(testing.AssertsCompiledSQL, BakedTest): run_setup_mappers = 'each' def _o2m_fixture(self, lazy="select", **kw): @@ -920,6 +922,72 @@ class LazyLoaderTest(BakedTest): sess.close() + def test_useget_cancels_eager(self): + """test that a one to many lazyload cancels the unnecessary + eager many-to-one join on the other side.""" + + User = self.classes.User + Address = self.classes.Address + + mapper(User, self.tables.users) + mapper(Address, self.tables.addresses, properties={ + 'user': relationship( + User, lazy='joined', + backref=backref('addresses', lazy='baked_select') + ) + }) + + sess = Session() + u1 = sess.query(User).filter(User.id == 8).one() + + def go(): + eq_(u1.addresses[0].user, u1) + self.assert_sql_execution( + testing.db, go, + CompiledSQL( + "SELECT addresses.id AS addresses_id, addresses.user_id AS " + "addresses_user_id, addresses.email_address AS " + "addresses_email_address FROM addresses WHERE :param_1 = " + "addresses.user_id", + {'param_1': 8}) + ) + + def test_useget_cancels_eager_propagated_present(self): + """test that a one to many lazyload cancels the unnecessary + eager many-to-one join on the other side, even when a propagated + option is present.""" + + User = self.classes.User + Address = self.classes.Address + + mapper(User, self.tables.users) + mapper(Address, self.tables.addresses, properties={ + 'user': relationship( + User, lazy='joined', + backref=backref('addresses', lazy='baked_select') + ) + }) + + from sqlalchemy.orm.interfaces import MapperOption + + class MyBogusOption(MapperOption): + propagate_to_loaders = True + + sess = Session() + u1 = sess.query(User).options(MyBogusOption()).filter(User.id == 8).one() + + def go(): + eq_(u1.addresses[0].user, u1) + self.assert_sql_execution( + testing.db, go, + CompiledSQL( + "SELECT addresses.id AS addresses_id, addresses.user_id AS " + "addresses_user_id, addresses.email_address AS " + "addresses_email_address FROM addresses WHERE :param_1 = " + "addresses.user_id", + {'param_1': 8}) + ) + # additional tests: # 1. m2m w lazyload # 2. o2m lazyload where m2o backrefs have an eager load, test diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index 759d4c5ba2..e57e2b3c2a 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -898,6 +898,43 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): {'param_1': 8}) ) + def test_useget_cancels_eager_propagated_present(self): + """test that a one to many lazyload cancels the unnecessary + eager many-to-one join on the other side, even when a propagated + option is present.""" + + users, Address, addresses, User = ( + self.tables.users, + self.classes.Address, + self.tables.addresses, + self.classes.User) + + mapper(User, users) + mapper(Address, addresses, properties={ + 'user': relationship(User, lazy='joined', backref='addresses') + }) + + from sqlalchemy.orm.interfaces import MapperOption + + class MyBogusOption(MapperOption): + propagate_to_loaders = True + + sess = create_session() + u1 = sess.query(User).options(MyBogusOption()).filter(User.id == 8).one() + + def go(): + eq_(u1.addresses[0].user, u1) + self.assert_sql_execution( + testing.db, go, + CompiledSQL( + "SELECT addresses.id AS addresses_id, addresses.user_id AS " + "addresses_user_id, addresses.email_address AS " + "addresses_email_address FROM addresses WHERE :param_1 = " + "addresses.user_id", + {'param_1': 8}) + ) + + def test_manytoone_limit(self): """test that the subquery wrapping only occurs with limit/offset and m2m or o2m joins present."""