Demote innerjoin to outerjoin coming from with_polymorphic

a with_polymorphic, regardless of inheritance type, represents
multiple classes.  A subclass that wants to joinedload with innerjoin=True
needs to be demoted to an outerjoin because the parent entity rows
might not be of that type.   Looks more intuitive with a joined
inheritance load, but applies just as well to single or concrete.

Change-Id: I4d3d76106ae20032269f8848aad70a8e2f9422f9
Fixes: #3988

6 files changed, 86 insertions, 9 deletions

diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst
index 24c525f85..3d0b7fa08 100644
--- a/doc/build/changelog/changelog_12.rst
+++ b/doc/build/changelog/changelog_12.rst
@@ -13,6 +13,17 @@
 .. changelog::
     :version: 1.2.0b1
 
+    .. change:: 3988
+        :tags: bug, orm
+        :tickets: 3988
+
+        Fixed bug where combining a "with_polymorphic" load in conjunction
+        with subclass-linked relationships that specify joinedload with
+        innerjoin=True, would fail to demote those "innerjoins" to
+        "outerjoins" to suit the other polymorphic classes that don't
+        support that relationship.   This applies to both a single and a
+        joined inheritance polymorphic load.
+
     .. change:: 3984
         :tags: bug, orm
         :tickets: 3984
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 3fdba4482..25aef3af7 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -687,6 +687,8 @@ class Mapper(InspectionAttr):
     is_mapper = True
     """Part of the inspection API."""
 
+    represents_outer_join = False
+
     @property
     def mapper(self):
         """Part of the inspection API.
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index a2f83818c..f1734194a 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -3729,7 +3729,8 @@ class _MapperEntity(_QueryEntity):
             self.path, adapter, context.primary_columns,
             with_polymorphic=self._with_polymorphic,
             only_load_props=query._only_load_props,
-            polymorphic_discriminator=self._polymorphic_discriminator)
+            polymorphic_discriminator=self._polymorphic_discriminator
+        )
 
     def __str__(self):
         return str(self.mapper)
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 00aabed0b..05bb55d58 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1502,7 +1502,9 @@ class JoinedLoader(AbstractRelationshipLoader):
 
         attach_on_outside = (
             not chained_from_outerjoin or
-            not innerjoin or innerjoin == 'unnested')
+            not innerjoin or innerjoin == 'unnested' or
+            entity.entity_zero.represents_outer_join
+        )
 
         if attach_on_outside:
             # this is the "classic" eager join case.
@@ -1510,7 +1512,9 @@ class JoinedLoader(AbstractRelationshipLoader):
                 towrap,
                 clauses.aliased_class,
                 onclause,
-                isouter=not innerjoin or (
+                isouter=not innerjoin or
+                entity.entity_zero.represents_outer_join or
+                (
                     chained_from_outerjoin and isinstance(towrap, sql.Join)
                 ), _left_memo=self.parent, _right_memo=self.mapper
             )
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 5bcb28f31..eebe18837 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -376,7 +376,8 @@ class AliasedClass(object):
                  with_polymorphic_mappers=(),
                  with_polymorphic_discriminator=None,
                  base_alias=None,
-                 use_mapper_path=False):
+                 use_mapper_path=False,
+                 represents_outer_join=False):
         mapper = _class_to_mapper(cls)
         if alias is None:
             alias = mapper._with_polymorphic_selectable.alias(
@@ -395,7 +396,8 @@ class AliasedClass(object):
             else mapper.polymorphic_on,
             base_alias,
             use_mapper_path,
-            adapt_on_names
+            adapt_on_names,
+            represents_outer_join
         )
 
         self.__name__ = 'AliasedClass_%s' % mapper.class_.__name__
@@ -478,7 +480,8 @@ class AliasedInsp(InspectionAttr):
 
     def __init__(self, entity, mapper, selectable, name,
                  with_polymorphic_mappers, polymorphic_on,
-                 _base_alias, _use_mapper_path, adapt_on_names):
+                 _base_alias, _use_mapper_path, adapt_on_names,
+                 represents_outer_join):
         self.entity = entity
         self.mapper = mapper
         self.selectable = selectable
@@ -487,6 +490,7 @@ class AliasedInsp(InspectionAttr):
         self.polymorphic_on = polymorphic_on
         self._base_alias = _base_alias or self
         self._use_mapper_path = _use_mapper_path
+        self.represents_outer_join = represents_outer_join
 
         self._adapter = sql_util.ColumnAdapter(
             selectable, equivalents=mapper._equivalent_columns,
@@ -530,7 +534,8 @@ class AliasedInsp(InspectionAttr):
             'with_polymorphic_discriminator':
                 self.polymorphic_on,
             'base_alias': self._base_alias,
-            'use_mapper_path': self._use_mapper_path
+            'use_mapper_path': self._use_mapper_path,
+            'represents_outer_join': self.represents_outer_join
         }
 
     def __setstate__(self, state):
@@ -543,7 +548,8 @@ class AliasedInsp(InspectionAttr):
             state['with_polymorphic_discriminator'],
             state['base_alias'],
             state['use_mapper_path'],
-            state['adapt_on_names']
+            state['adapt_on_names'],
+            state['represents_outer_join']
         )
 
     def _adapt_element(self, elem):
@@ -772,7 +778,8 @@ def with_polymorphic(base, classes, selectable=False,
                         selectable,
                         with_polymorphic_mappers=mappers,
                         with_polymorphic_discriminator=polymorphic_on,
-                        use_mapper_path=_use_mapper_path)
+                        use_mapper_path=_use_mapper_path,
+                        represents_outer_join=not innerjoin)
 
 
 def _orm_annotate(element, exclude=None):
diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py
index 74956930c..246cf214b 100644
--- a/test/orm/inheritance/test_relationship.py
+++ b/test/orm/inheritance/test_relationship.py
@@ -1662,6 +1662,58 @@ class JoinedloadOverWPolyAliased(
             "LEFT OUTER JOIN link AS link_2 ON parent_1.id = link_2.parent_id"
         )
 
+    def test_local_wpoly_innerjoins(self):
+        # test for issue #3988
+        Sub1 = self._fixture_from_subclass()
+        Parent = self.classes.Parent
+        Link = self.classes.Link
+
+        poly = with_polymorphic(Parent, [Sub1])
+
+        session = Session()
+        q = session.query(poly).options(
+            joinedload(poly.Sub1.links, innerjoin=True).
+            joinedload(Link.child.of_type(Sub1), innerjoin=True).
+            joinedload(poly.Sub1.links, innerjoin=True)
+        )
+        self.assert_compile(
+            q,
+            "SELECT parent.id AS parent_id, parent.type AS parent_type, "
+            "link_1.parent_id AS link_1_parent_id, "
+            "link_1.child_id AS link_1_child_id, "
+            "parent_1.id AS parent_1_id, parent_1.type AS parent_1_type, "
+            "link_2.parent_id AS link_2_parent_id, "
+            "link_2.child_id AS link_2_child_id FROM parent "
+            "LEFT OUTER JOIN link AS link_1 ON parent.id = link_1.parent_id "
+            "LEFT OUTER JOIN parent AS parent_1 "
+            "ON link_1.child_id = parent_1.id "
+            "LEFT OUTER JOIN link AS link_2 ON parent_1.id = link_2.parent_id"
+        )
+
+    def test_local_wpoly_innerjoins_roundtrip(self):
+        # test for issue #3988
+        Sub1 = self._fixture_from_subclass()
+        Parent = self.classes.Parent
+        Link = self.classes.Link
+
+        session = Session()
+        session.add_all([
+            Parent(),
+            Parent()
+        ])
+
+        # represents "Parent" and "Sub1" rows
+        poly = with_polymorphic(Parent, [Sub1])
+
+        # innerjoin for Sub1 only, but this needs
+        # to be cancelled because the Parent rows
+        # would be omitted
+        q = session.query(poly).options(
+            joinedload(poly.Sub1.links, innerjoin=True).
+            joinedload(Link.child.of_type(Sub1), innerjoin=True)
+        )
+        eq_(len(q.all()), 2)
+
 
 class JoinAcrossJoinedInhMultiPath(fixtures.DeclarativeMappedTest,
                                    testing.AssertsCompiledSQL):