summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2016-10-14 17:06:07 -0400
committerMike Bayer <mike_mp@zzzcomputing.com>2016-10-17 11:28:36 -0400
commitae7d2837b3c5ae3fd6e9dad6b14a26abb32cfee5 (patch)
tree364dc06116db3131c4dc0f70ee365b1b440f6be7
parent4684cfb50836dc57107e49d4a78a8889c40d9662 (diff)
downloadsqlalchemy-ae7d2837b3c5ae3fd6e9dad6b14a26abb32cfee5.tar.gz
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
-rw-r--r--doc/build/changelog/changelog_11.rst8
-rw-r--r--lib/sqlalchemy/ext/baked.py8
-rw-r--r--lib/sqlalchemy/orm/strategies.py5
-rw-r--r--lib/sqlalchemy/orm/strategy_options.py8
-rw-r--r--test/ext/test_baked.py72
-rw-r--r--test/orm/test_eager_relations.py37
6 files changed, 133 insertions, 5 deletions
diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index b90b932b5..127227ded 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 3ca94925e..2f658edf3 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 b2cd5b5ec..5f5ab1069 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 2fb13f3cf..0c1ebf404 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 2f501eb6c..818019de2 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 759d4c5ba..e57e2b3c2 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."""