Establish that contains_eager()->alias can be replaced by of_type

One test in test_of_type was creating a cartesian product
because contains_eager() was used with "alias" to refer
to a with_polymorphic(), but the wp was not used with of_type(),
so the pathing did not know that additional entities were present.

while the docs indicate that of_type() should be used, there is no
reason to use "alias" when you are using of_type().   Attempts
to make this automatic don't work as the current usage contract
with "alias" is that the contains_eager() chain can continue
along in terms of the base entities, which is another example
of the implicit swapping of entities for an aliased version of
themselves that really should be entirely marked as deprecated
throughout 1.4 and removed in 2.0.

So instead, add test coverage for the of_type() versions of
things and begin to make the case that we can remove "alias"
entirely, where previously we thought we would only deprecate
the string form.

For the 1.3 backport we are also picking a little bit of the
lambda combinations improvements that landed in 217948f5c7 and for
some reason were not backported.

Fixes: #5096
Change-Id: Ia7b021c4044332ab3282267815f208da64410e95
(cherry picked from commit b54b6ff7c09d15cedec3d65b10cd383ef41f1fbc)

diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index c9372836306c2fb4ebd7443d858f56d9438b47d3..848d47da0e3c5cc0f84745d17bbd80b59fdf2092 100644 (file)
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -414,7 +414,8 @@ class PropComparator(operators.ColumnOperators):
         return a.of_type(class_)
 
     def of_type(self, class_):
-        r"""Redefine this object in terms of a polymorphic subclass.
+        r"""Redefine this object in terms of a polymorphic subclass,
+        :func:`.with_polymorphic` construct, or :func:`.aliased` construct.
 
         Returns a new PropComparator from which further criterion can be
         evaluated.

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index d9c12ac21e61057c587a00c4e7f9726a158df223..b298081864c5e1c19de920e46647322b3debc0a5 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1006,28 +1006,41 @@ def contains_eager(loadopt, attr, alias=None):
     ``User`` entity, and the returned ``Order`` objects would have the
     ``Order.user`` attribute pre-populated.
 
-    :func:`.contains_eager` also accepts an `alias` argument, which is the
-    string name of an alias, an :func:`~sqlalchemy.sql.expression.alias`
-    construct, or an :func:`~sqlalchemy.orm.aliased` construct. Use this when
-    the eagerly-loaded rows are to come from an aliased table::
+    When making use of aliases with :func:`.contains_eager`, the path
+    should be specified using :meth:`.PropComparator.of_type`::
 
         user_alias = aliased(User)
         sess.query(Order).\
                 join((user_alias, Order.user)).\
-                options(contains_eager(Order.user, alias=user_alias))
+                options(contains_eager(Order.user.of_type(user_alias)))
 
-    When using :func:`.contains_eager` in conjunction with inherited
-    subclasses, the :meth:`.RelationshipProperty.of_type` modifier should
-    also be used in order to set up the pathing properly::
+    :meth:`.PropComparator.of_type` is also used to indicate a join
+    against specific subclasses of an inherting mapper, or
+    of a :func:`.with_polymorphic` construct::
 
+        # employees of a particular subtype
         sess.query(Company).\
             outerjoin(Company.employees.of_type(Manager)).\
             options(
                 contains_eager(
                     Company.employees.of_type(Manager),
-                    alias=Manager)
+                )
+            )
+
+        # employees of a multiple subtypes
+        wp = with_polymorphic(Employee, [Manager, Engineer])
+        sess.query(Company).\
+            outerjoin(Company.employees.of_type(wp)).\
+            options(
+                contains_eager(
+                    Company.employees.of_type(wp),
+                )
             )
 
+    The :paramref:`.contains_eager.alias` parameter is used for a similar
+    purpose, however the :meth:`.PropComparator.of_type` approach should work
+    in all cases and is more effective and explicit.
+
     .. seealso::
 
         :ref:`loading_toplevel`

diff --git a/lib/sqlalchemy/testing/util.py b/lib/sqlalchemy/testing/util.py
index bde2ca54a2da698d03ae204ee38fa7a255dcaaab..df63e565d8cae2779aa114721155bc3026ab55b2 100644 (file)
--- a/lib/sqlalchemy/testing/util.py
+++ b/lib/sqlalchemy/testing/util.py
@@ -14,6 +14,7 @@ import types
 
 from ..util import decorator
 from ..util import defaultdict
+from ..util import inspect_getfullargspec
 from ..util import jython
 from ..util import py2k
 from ..util import pypy
@@ -270,10 +271,12 @@ def resolve_lambda(__fn, **kw):
 
     """
 
+    pos_args = inspect_getfullargspec(__fn)[0]
+    pass_pos_args = {arg: kw.pop(arg) for arg in pos_args}
     glb = dict(__fn.__globals__)
     glb.update(kw)
     new_fn = types.FunctionType(__fn.__code__, glb)
-    return new_fn()
+    return new_fn(**pass_pos_args)
 
 
 def metadata_fixture(ddl="function"):

diff --git a/test/orm/inheritance/_poly_fixtures.py b/test/orm/inheritance/_poly_fixtures.py
index cc253a3806dcd17eeb1646530feed3353de2533d..1aa5b68fb66e93cc98223d4d4e744fd6c9912cfb 100644 (file)
--- a/test/orm/inheritance/_poly_fixtures.py
+++ b/test/orm/inheritance/_poly_fixtures.py
@@ -1,6 +1,7 @@
 from sqlalchemy import ForeignKey
 from sqlalchemy import Integer
 from sqlalchemy import String
+from sqlalchemy import util
 from sqlalchemy.orm import create_session
 from sqlalchemy.orm import mapper
 from sqlalchemy.orm import polymorphic_union
@@ -384,11 +385,14 @@ class _PolymorphicUnions(_PolymorphicFixtureBase):
             cls.tables.managers,
             cls.tables.boss,
         )
+
         person_join = polymorphic_union(
-            {
-                "engineer": people.join(engineers),
-                "manager": people.join(managers),
-            },
+            util.OrderedDict(
+                [
+                    ("engineer", people.join(engineers)),
+                    ("manager", people.join(managers)),
+                ]
+            ),
             None,
             "pjoin",
         )

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 56beb8c1c4d4327869cbedf97e29fd04d644ff6f..a5953452ae0a992c1d2dc1ae4a459e8450f5de55 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -5383,6 +5383,35 @@ class EntityViaMultiplePathTestOne(fixtures.DeclarativeMappedTest):
         # PYTHONHASHSEED
         in_("d", a1.c.__dict__)
 
+    def test_multi_path_load_of_type(self):
+        A, B, C, D = self.classes("A", "B", "C", "D")
+
+        s = Session()
+
+        c = C(d=D())
+
+        s.add(A(b=B(c=c), c=c))
+        s.commit()
+
+        c_alias_1 = aliased(C)
+        c_alias_2 = aliased(C)
+
+        q = s.query(A)
+        q = q.join(A.b).join(B.c.of_type(c_alias_1)).join(c_alias_1.d)
+        q = q.options(
+            contains_eager(A.b)
+            .contains_eager(B.c.of_type(c_alias_1))
+            .contains_eager(c_alias_1.d)
+        )
+        q = q.join(A.c.of_type(c_alias_2))
+        q = q.options(contains_eager(A.c.of_type(c_alias_2)))
+
+        a1 = q.all()[0]
+
+        # ensure 'd' key was populated in dict.  Varies based on
+        # PYTHONHASHSEED
+        in_("d", a1.c.__dict__)
+
 
 class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest):
     """test for [ticket:3431]"""
@@ -5441,6 +5470,7 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest):
         l_ac = aliased(LD)
         u_ac = aliased(User)
 
+        # these paths don't work out correctly?
         lz_test = (
             s.query(LDA)
             .join("ld")
@@ -5456,6 +5486,39 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest):
 
         in_("user", lz_test.a.ld.__dict__)
 
+    def test_multi_path_load_of_type(self):
+        User, LD, A, LDA = self.classes("User", "LD", "A", "LDA")
+
+        s = Session()
+
+        u0 = User(data=42)
+        l0 = LD(user=u0)
+        z0 = A(ld=l0)
+        lz0 = LDA(ld=l0, a=z0)
+        s.add_all([u0, l0, z0, lz0])
+        s.commit()
+
+        l_ac = aliased(LD)
+        u_ac = aliased(User)
+
+        lz_test = (
+            s.query(LDA)
+            .join(LDA.ld)
+            .options(contains_eager(LDA.ld))
+            .join(LDA.a)
+            .join(LDA.ld.of_type(l_ac))
+            .join(l_ac.user.of_type(u_ac))
+            .options(
+                contains_eager(LDA.a),
+                contains_eager(LDA.ld.of_type(l_ac)).contains_eager(
+                    l_ac.user.of_type(u_ac)
+                ),
+            )
+            .first()
+        )
+
+        in_("user", lz_test.a.ld.__dict__)
+
 
 class LazyLoadOptSpecificityTest(fixtures.DeclarativeMappedTest):
     """test for [ticket:3963]"""

diff --git a/test/orm/test_froms.py b/test/orm/test_froms.py
index ec6b9a906ac3078b69f73567ade14497141705e9..72358553ed342b62f89227d79c88d426a697af3f 100644 (file)
--- a/test/orm/test_froms.py
+++ b/test/orm/test_froms.py
@@ -1235,6 +1235,36 @@ class InstancesTest(QueryTest, AssertsCompiledSQL):
 
         self.assert_sql_count(testing.db, go, 1)
 
+    def test_contains_eager_multi_aliased_of_type(self):
+        # test newer style that does not use the alias parameter
+        Item, User, Order = (
+            self.classes.Item,
+            self.classes.User,
+            self.classes.Order,
+        )
+
+        sess = create_session()
+        q = sess.query(User)
+
+        # test using Aliased with more than one level deep
+        oalias = aliased(Order)
+        ialias = aliased(Item)
+
+        def go():
+            result = (
+                q.options(
+                    contains_eager(User.orders.of_type(oalias)).contains_eager(
+                        oalias.items.of_type(ialias)
+                    )
+                )
+                .outerjoin(User.orders.of_type(oalias))
+                .outerjoin(oalias.items.of_type(ialias))
+                .order_by(User.id, oalias.id, ialias.id)
+            )
+            assert self.static.user_order_result == result.all()
+
+        self.assert_sql_count(testing.db, go, 1)
+
     def test_contains_eager_chaining(self):
         """test that contains_eager() 'chains' by default."""
 

diff --git a/test/orm/test_of_type.py b/test/orm/test_of_type.py
index c170026fa0269361962e7b46896b8d14b2b561ab..e49ea34495f2db63a69b219c403db36bc2d7dc9f 100644 (file)
--- a/test/orm/test_of_type.py
+++ b/test/orm/test_of_type.py
@@ -152,22 +152,41 @@ class _PolymorphicTestBase(object):
 
         self.assert_sql_count(testing.db, go, 1)
 
-    def test_with_polymorphic_join_exec_contains_eager_two(self):
+    @testing.combinations(
+        # this form is not expected to work in all cases, ultimately
+        # the "alias" parameter should be deprecated entirely
+        # lambda Company, wp: contains_eager(Company.employees, alias=wp),
+        lambda Company, wp: contains_eager(Company.employees.of_type(wp)),
+        lambda Company, wp: contains_eager(
+            Company.employees.of_type(wp), alias=wp
+        ),
+    )
+    def test_with_polymorphic_join_exec_contains_eager_two(
+        self, contains_eager_option
+    ):
         sess = Session()
 
+        wp = with_polymorphic(Person, [Engineer, Manager], aliased=True)
+        contains_eager_option = testing.resolve_lambda(
+            contains_eager_option, Company=Company, wp=wp
+        )
+        q = (
+            sess.query(Company)
+            .join(Company.employees.of_type(wp))
+            .order_by(Company.company_id, wp.person_id)
+            .options(contains_eager_option)
+        )
+
         def go():
-            wp = with_polymorphic(Person, [Engineer, Manager], aliased=True)
-            eq_(
-                sess.query(Company)
-                .join(Company.employees.of_type(wp))
-                .order_by(Company.company_id, wp.person_id)
-                .options(contains_eager(Company.employees, alias=wp))
-                .all(),
-                [self.c1, self.c2],
-            )
+            eq_(q.all(), [self.c1, self.c2])
 
         self.assert_sql_count(testing.db, go, 1)
 
+        self.assert_compile(
+            q,
+            self._test_with_polymorphic_join_exec_contains_eager_two_result(),
+        )
+
     def test_with_polymorphic_any(self):
         sess = Session()
         wp = with_polymorphic(Person, [Engineer], aliased=True)
@@ -275,6 +294,38 @@ class PolymorphicPolymorphicTest(
             + " ON companies.company_id = people_1.company_id"
         )
 
+    def _test_with_polymorphic_join_exec_contains_eager_two_result(self):
+        return (
+            "SELECT anon_1.people_person_id AS anon_1_people_person_id, "
+            "anon_1.people_company_id AS anon_1_people_company_id, "
+            "anon_1.people_name AS anon_1_people_name, "
+            "anon_1.people_type AS anon_1_people_type, "
+            "anon_1.engineers_person_id AS anon_1_engineers_person_id, "
+            "anon_1.engineers_status AS anon_1_engineers_status, "
+            "anon_1.engineers_engineer_name "
+            "AS anon_1_engineers_engineer_name, "
+            "anon_1.engineers_primary_language "
+            "AS anon_1_engineers_primary_language, anon_1.managers_person_id "
+            "AS anon_1_managers_person_id, anon_1.managers_status "
+            "AS anon_1_managers_status, anon_1.managers_manager_name "
+            "AS anon_1_managers_manager_name, companies.company_id "
+            "AS companies_company_id, companies.name AS companies_name "
+            "FROM companies JOIN (SELECT people.person_id AS "
+            "people_person_id, people.company_id AS people_company_id, "
+            "people.name AS people_name, people.type AS people_type, "
+            "engineers.person_id AS engineers_person_id, engineers.status "
+            "AS engineers_status, engineers.engineer_name "
+            "AS engineers_engineer_name, engineers.primary_language "
+            "AS engineers_primary_language, managers.person_id "
+            "AS managers_person_id, managers.status AS managers_status, "
+            "managers.manager_name AS managers_manager_name FROM people "
+            "LEFT OUTER JOIN engineers ON people.person_id = "
+            "engineers.person_id LEFT OUTER JOIN managers "
+            "ON people.person_id = managers.person_id) AS anon_1 "
+            "ON companies.company_id = anon_1.people_company_id "
+            "ORDER BY companies.company_id, anon_1.people_person_id"
+        )
+
 
 class PolymorphicUnionsTest(_PolymorphicTestBase, _PolymorphicUnions):
     def _polymorphic_join_target(self, cls):
@@ -288,6 +339,34 @@ class PolymorphicUnionsTest(_PolymorphicTestBase, _PolymorphicUnions):
             + " AS anon_1 ON companies.company_id = anon_1.company_id"
         )
 
+    def _test_with_polymorphic_join_exec_contains_eager_two_result(self):
+        return (
+            "SELECT anon_1.person_id AS anon_1_person_id, "
+            "anon_1.company_id AS anon_1_company_id, anon_1.name AS "
+            "anon_1_name, anon_1.type AS anon_1_type, anon_1.status "
+            "AS anon_1_status, anon_1.engineer_name AS "
+            "anon_1_engineer_name, anon_1.primary_language AS "
+            "anon_1_primary_language, anon_1.manager_name AS "
+            "anon_1_manager_name, companies.company_id AS "
+            "companies_company_id, companies.name AS companies_name "
+            "FROM companies JOIN (SELECT engineers.person_id AS "
+            "person_id, people.company_id AS company_id, people.name AS name, "
+            "people.type AS type, engineers.status AS status, "
+            "engineers.engineer_name AS engineer_name, "
+            "engineers.primary_language AS primary_language, "
+            "CAST(NULL AS VARCHAR(50)) AS manager_name FROM people "
+            "JOIN engineers ON people.person_id = engineers.person_id "
+            "UNION ALL SELECT managers.person_id AS person_id, "
+            "people.company_id AS company_id, people.name AS name, "
+            "people.type AS type, managers.status AS status, "
+            "CAST(NULL AS VARCHAR(50)) AS engineer_name, "
+            "CAST(NULL AS VARCHAR(50)) AS primary_language, "
+            "managers.manager_name AS manager_name FROM people "
+            "JOIN managers ON people.person_id = managers.person_id) AS "
+            "anon_1 ON companies.company_id = anon_1.company_id "
+            "ORDER BY companies.company_id, anon_1.person_id"
+        )
+
 
 class PolymorphicAliasedJoinsTest(
     _PolymorphicTestBase, _PolymorphicAliasedJoins
@@ -303,6 +382,37 @@ class PolymorphicAliasedJoinsTest(
             + " AS anon_1 ON companies.company_id = anon_1.people_company_id"
         )
 
+    def _test_with_polymorphic_join_exec_contains_eager_two_result(self):
+        return (
+            "SELECT anon_1.people_person_id AS anon_1_people_person_id, "
+            "anon_1.people_company_id AS anon_1_people_company_id, "
+            "anon_1.people_name AS anon_1_people_name, anon_1.people_type "
+            "AS anon_1_people_type, anon_1.engineers_person_id AS "
+            "anon_1_engineers_person_id, anon_1.engineers_status AS "
+            "anon_1_engineers_status, anon_1.engineers_engineer_name "
+            "AS anon_1_engineers_engineer_name, "
+            "anon_1.engineers_primary_language AS "
+            "anon_1_engineers_primary_language, anon_1.managers_person_id "
+            "AS anon_1_managers_person_id, anon_1.managers_status "
+            "AS anon_1_managers_status, anon_1.managers_manager_name "
+            "AS anon_1_managers_manager_name, companies.company_id "
+            "AS companies_company_id, companies.name AS companies_name "
+            "FROM companies JOIN (SELECT people.person_id AS "
+            "people_person_id, people.company_id AS people_company_id, "
+            "people.name AS people_name, people.type AS people_type, "
+            "engineers.person_id AS engineers_person_id, engineers.status "
+            "AS engineers_status, engineers.engineer_name AS "
+            "engineers_engineer_name, engineers.primary_language "
+            "AS engineers_primary_language, managers.person_id AS "
+            "managers_person_id, managers.status AS managers_status, "
+            "managers.manager_name AS managers_manager_name FROM people "
+            "LEFT OUTER JOIN engineers ON people.person_id = "
+            "engineers.person_id LEFT OUTER JOIN managers "
+            "ON people.person_id = managers.person_id) AS anon_1 "
+            "ON companies.company_id = anon_1.people_company_id "
+            "ORDER BY companies.company_id, anon_1.people_person_id"
+        )
+
 
 class PolymorphicJoinsTest(_PolymorphicTestBase, _PolymorphicJoins):
     def _polymorphic_join_target(self, cls):
@@ -319,6 +429,38 @@ class PolymorphicJoinsTest(_PolymorphicTestBase, _PolymorphicJoins):
             + " ON companies.company_id = people_1.company_id"
         )
 
+    def _test_with_polymorphic_join_exec_contains_eager_two_result(self):
+        return (
+            "SELECT anon_1.people_person_id AS anon_1_people_person_id, "
+            "anon_1.people_company_id AS anon_1_people_company_id, "
+            "anon_1.people_name AS anon_1_people_name, "
+            "anon_1.people_type AS anon_1_people_type, "
+            "anon_1.engineers_person_id AS anon_1_engineers_person_id, "
+            "anon_1.engineers_status AS anon_1_engineers_status, "
+            "anon_1.engineers_engineer_name "
+            "AS anon_1_engineers_engineer_name, "
+            "anon_1.engineers_primary_language "
+            "AS anon_1_engineers_primary_language, anon_1.managers_person_id "
+            "AS anon_1_managers_person_id, anon_1.managers_status "
+            "AS anon_1_managers_status, anon_1.managers_manager_name "
+            "AS anon_1_managers_manager_name, companies.company_id "
+            "AS companies_company_id, companies.name AS companies_name "
+            "FROM companies JOIN (SELECT people.person_id AS "
+            "people_person_id, people.company_id AS people_company_id, "
+            "people.name AS people_name, people.type AS people_type, "
+            "engineers.person_id AS engineers_person_id, engineers.status "
+            "AS engineers_status, engineers.engineer_name "
+            "AS engineers_engineer_name, engineers.primary_language "
+            "AS engineers_primary_language, managers.person_id "
+            "AS managers_person_id, managers.status AS managers_status, "
+            "managers.manager_name AS managers_manager_name FROM people "
+            "LEFT OUTER JOIN engineers ON people.person_id = "
+            "engineers.person_id LEFT OUTER JOIN managers "
+            "ON people.person_id = managers.person_id) AS anon_1 "
+            "ON companies.company_id = anon_1.people_company_id "
+            "ORDER BY companies.company_id, anon_1.people_person_id"
+        )
+
     def test_joinedload_explicit_with_unaliased_poly_compile(self):
         sess = Session()
         target = with_polymorphic(Person, Engineer)