Adjust use_mapper_path rule for poly subclasses

We must change the approach from 2734439 as the information
loss is breaking subquery eager loading.

Move the adjustment into a deeper set of logic inside
of path_regsitry.  We can distinguish between a path
that will "naturally" build from an aliased entity
at the base, vs. one that will "naturally" build
on all raw mappers, based on if when we observe that
we are being given a with_polymorphic(), if the existing
parent path is already in progress or not.

In general, we prefer paths to have as much of the original
information as possible, and the "natural path" is supposed
to be where the loader lookup stuff happens.

Fixes: #5082
Change-Id: I3c0ee72993bae8a6f067bdef3dc9a57d83f64950

2 files changed, 47 insertions, 7 deletions

diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py
index 1de54251c..2df1b3b4c 100644
--- a/lib/sqlalchemy/orm/path_registry.py
+++ b/lib/sqlalchemy/orm/path_registry.py
@@ -243,27 +243,63 @@ class TokenRegistry(PathRegistry):
 
 
 class PropRegistry(PathRegistry):
+    is_unnatural = False
+
     def __init__(self, parent, prop):
         # restate this path in terms of the
         # given MapperProperty's parent.
         insp = inspection.inspect(parent[-1])
+        natural_parent = parent
         if not insp.is_aliased_class or insp._use_mapper_path:
-            parent = parent.parent[prop.parent]
+            parent = natural_parent = parent.parent[prop.parent]
         elif insp.is_aliased_class and insp.with_polymorphic_mappers:
             if (
                 prop.parent is not insp.mapper
                 and prop.parent in insp.with_polymorphic_mappers
             ):
                 subclass_entity = parent[-1]._entity_for_mapper(prop.parent)
-                if subclass_entity._use_mapper_path:
-                    parent = parent.parent[subclass_entity.mapper]
+                parent = parent.parent[subclass_entity]
+
+                # when building a path where with_polymorphic() is in use,
+                # special logic to determine the "natural path" when subclass
+                # entities are used.
+                #
+                # here we are trying to distinguish between a path that starts
+                # on a the with_polymorhpic entity vs. one that starts on a
+                # normal entity that introduces a with_polymorphic() in the
+                # middle using of_type():
+                #
+                #  # as in test_polymorphic_rel->
+                #  #    test_subqueryload_on_subclass_uses_path_correctly
+                #  wp = with_polymorphic(RegularEntity, "*")
+                #  sess.query(wp).options(someload(wp.SomeSubEntity.foos))
+                #
+                # vs
+                #
+                #  # as in test_relationship->JoinedloadWPolyOfTypeContinued
+                #  wp = with_polymorphic(SomeFoo, "*")
+                #  sess.query(RegularEntity).options(
+                #       someload(RegularEntity.foos.of_type(wp))
+                #       .someload(wp.SubFoo.bar)
+                #   )
+                #
+                # in the former case, the Query as it generates a path that we
+                # want to match will be in terms of the with_polymorphic at the
+                # beginning.  in the latter case, Query will generate simple
+                # paths that don't know about this with_polymorphic, so we must
+                # use a separate natural path.
+                #
+                #
+                if parent.parent:
+                    natural_parent = parent.parent[subclass_entity.mapper]
+                    self.is_unnatural = True
                 else:
-                    parent = parent.parent[subclass_entity]
+                    natural_parent = parent
 
         self.prop = prop
         self.parent = parent
         self.path = parent.path + (prop,)
-        self.natural_path = parent.natural_path + (prop,)
+        self.natural_path = natural_parent.natural_path + (prop,)
 
         self._wildcard_path_loader_key = (
             "loader",
@@ -319,7 +355,11 @@ class AbstractEntityRegistry(PathRegistry):
         # "enhanced" path in self.path and the "natural" path that doesn't
         # include those objects so these two traversals can be matched up.
 
-        if parent.path and self.is_aliased_class:
+        # the test here for "(self.is_aliased_class or parent.is_unnatural)"
+        # are to avoid the more expensive conditional logic that follows if we
+        # know we don't have to do it.   This conditional can just as well be
+        # "if parent.path:", it just is more function calls.
+        if parent.path and (self.is_aliased_class or parent.is_unnatural):
             # this is an infrequent code path used only for loader strategies
             # that also make use of of_type().
             if entity.mapper.isa(parent.natural_path[-1].entity):
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 3a82de781..ccadeeaac 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -606,7 +606,7 @@ class AliasedInsp(sql_base.HasCacheKey, InspectionAttr):
                         selectable,
                         base_alias=self,
                         adapt_on_names=adapt_on_names,
-                        use_mapper_path=True,
+                        use_mapper_path=_use_mapper_path,
                     )
 
                     setattr(self.entity, poly.class_.__name__, ent)