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.

Fixes: #5096
Change-Id: Ia7b021c4044332ab3282267815f208da64410e95

diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index c7b059fda18551d7870c3d7b316202d833f67360..f75c7d3bac27ad5c770139212e74ee632a6e7f46 100644 (file)
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -435,7 +435,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 b123ecc5ddec0dcaa1791517398050a3f4d323af..0c72f3b37dcec3bceb5ac8059a1611e6ff8f909d 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1037,28 +1037,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`
@@ -1070,6 +1083,7 @@ def contains_eager(loadopt, attr, alias=None):
         if not isinstance(alias, str):
             info = inspect(alias)
             alias = info.selectable
+
         else:
             util.warn_deprecated(
                 "Passing a string name for the 'alias' argument to "

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index a33da3512aa7d79c455fba005f2483ff8931b14b..659f6e103e0a4b6747680e247cd80750fa39a65a 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -5399,6 +5399,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]"""
@@ -5457,6 +5486,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")
@@ -5472,6 +5502,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 b22bb1601a7816ebaefe3dd826235205dede0103..f3fda579aea2a884ba99df942aa9561a0234d199 100644 (file)
--- a/test/orm/test_froms.py
+++ b/test/orm/test_froms.py
@@ -1109,6 +1109,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 a431d231f4da7b45225b0f93b04d79d32d15ee12..d6cbcf12016626368d322204e4c76f1f29555d1d 100644 (file)
--- a/test/orm/test_of_type.py
+++ b/test/orm/test_of_type.py
@@ -151,22 +151,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)
@@ -269,6 +288,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):
@@ -290,6 +341,34 @@ class PolymorphicUnionsTest(_PolymorphicTestBase, _PolymorphicUnions):
             "AS pjoin_1 ON companies.company_id = pjoin_1.company_id"
         )
 
+    def _test_with_polymorphic_join_exec_contains_eager_two_result(self):
+        return (
+            "SELECT pjoin_1.person_id AS pjoin_1_person_id, "
+            "pjoin_1.company_id AS pjoin_1_company_id, pjoin_1.name AS "
+            "pjoin_1_name, pjoin_1.type AS pjoin_1_type, pjoin_1.status "
+            "AS pjoin_1_status, pjoin_1.engineer_name AS "
+            "pjoin_1_engineer_name, pjoin_1.primary_language AS "
+            "pjoin_1_primary_language, pjoin_1.manager_name AS "
+            "pjoin_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 "
+            "pjoin_1 ON companies.company_id = pjoin_1.company_id "
+            "ORDER BY companies.company_id, pjoin_1.person_id"
+        )
+
 
 class PolymorphicAliasedJoinsTest(
     _PolymorphicTestBase, _PolymorphicAliasedJoins
@@ -313,6 +392,37 @@ class PolymorphicAliasedJoinsTest(
             "ON companies.company_id = pjoin_1.people_company_id"
         )
 
+    def _test_with_polymorphic_join_exec_contains_eager_two_result(self):
+        return (
+            "SELECT pjoin_1.people_person_id AS pjoin_1_people_person_id, "
+            "pjoin_1.people_company_id AS pjoin_1_people_company_id, "
+            "pjoin_1.people_name AS pjoin_1_people_name, pjoin_1.people_type "
+            "AS pjoin_1_people_type, pjoin_1.engineers_person_id AS "
+            "pjoin_1_engineers_person_id, pjoin_1.engineers_status AS "
+            "pjoin_1_engineers_status, pjoin_1.engineers_engineer_name "
+            "AS pjoin_1_engineers_engineer_name, "
+            "pjoin_1.engineers_primary_language AS "
+            "pjoin_1_engineers_primary_language, pjoin_1.managers_person_id "
+            "AS pjoin_1_managers_person_id, pjoin_1.managers_status "
+            "AS pjoin_1_managers_status, pjoin_1.managers_manager_name "
+            "AS pjoin_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 pjoin_1 "
+            "ON companies.company_id = pjoin_1.people_company_id "
+            "ORDER BY companies.company_id, pjoin_1.people_person_id"
+        )
+
 
 class PolymorphicJoinsTest(_PolymorphicTestBase, _PolymorphicJoins):
     def _polymorphic_join_target(self, cls):
@@ -324,6 +434,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)