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

2 files changed, 25 insertions, 10 deletions

diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index c7b059fda..f75c7d3ba 100644
--- 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 b123ecc5d..0c72f3b37 100644
--- 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 "