From b72c14cde21abbc4a8cea9089853c4f34ed2e364 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 27 Apr 2020 16:51:43 -0400
Subject: [PATCH] Alias the onclause if ORM join is to same polymorphic
 selectable

Fixed bug where using :func:`.with_polymorphic` as the target of a join via
:meth:`.RelationshipComparator.of_type` on a mapper that already has a
subquery-based with_polymorphic setting that's equivalent to the one
requested would not correctly alias the ON clause in the join.

Fixes: #5288
Change-Id: I0212a990ee67a344c87fe21833bf47fdb72ca0cc
(cherry picked from commit 483a644959f396e3abdcb8f0f373936569958970)
---
 doc/build/changelog/unreleased_13/5288.rst   |  8 ++++
 lib/sqlalchemy/orm/query.py                  |  1 -
 lib/sqlalchemy/orm/relationships.py          |  5 +-
 test/orm/inheritance/_poly_fixtures.py       | 10 ++++
 test/orm/inheritance/test_polymorphic_rel.py | 49 ++++++++++++++++++++
 5 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 doc/build/changelog/unreleased_13/5288.rst

diff --git a/doc/build/changelog/unreleased_13/5288.rst b/doc/build/changelog/unreleased_13/5288.rst
new file mode 100644
index 0000000000..d5fbdf5af1
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5288.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 5288
+
+    Fixed bug where using :func:`.with_polymorphic` as the target of a join via
+    :meth:`.RelationshipComparator.of_type` on a mapper that already has a
+    subquery-based with_polymorphic setting that's equivalent to the one
+    requested would not correctly alias the ON clause in the join.
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 1bcf498421..2cf82c7d49 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -2849,7 +2849,6 @@ class Query(object):
             elif prop:
                 # joining to selectable with a mapper property given
                 # as the ON clause
-
                 if not right_selectable.is_derived_from(
                     right_mapper.persist_selectable
                 ):
diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index daa85c5661..209308e4db 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -2361,7 +2361,10 @@ class RelationshipProperty(StrategizedProperty):
             if self._is_self_referential and source_selectable is None:
                 dest_selectable = dest_selectable.alias()
                 aliased = True
-        elif dest_selectable is not self.mapper._with_polymorphic_selectable:
+        elif (
+            dest_selectable is not self.mapper._with_polymorphic_selectable
+            or self.mapper.with_polymorphic
+        ):
             aliased = True
 
         dest_mapper = of_type_mapper or self.mapper
diff --git a/test/orm/inheritance/_poly_fixtures.py b/test/orm/inheritance/_poly_fixtures.py
index 4756ae78ad..24a600fa50 100644
--- a/test/orm/inheritance/_poly_fixtures.py
+++ b/test/orm/inheritance/_poly_fixtures.py
@@ -150,6 +150,16 @@ class _PolymorphicFixtureBase(fixtures.MappedTest, AssertsCompiledSQL):
             Column("person_id", Integer, ForeignKey("people.person_id")),
         )
 
+    @classmethod
+    def setup_classes(cls):
+        cls.classes["Engineer"] = Engineer
+        cls.classes["Person"] = Person
+        cls.classes["Manager"] = Manager
+        cls.classes["Machine"] = Machine
+        cls.classes["Boss"] = Boss
+        cls.classes["Company"] = Company
+        cls.classes["Paperwork"] = Paperwork
+
     @classmethod
     def insert_data(cls, connection):
 
diff --git a/test/orm/inheritance/test_polymorphic_rel.py b/test/orm/inheritance/test_polymorphic_rel.py
index 880cc5f913..ad9b570150 100644
--- a/test/orm/inheritance/test_polymorphic_rel.py
+++ b/test/orm/inheritance/test_polymorphic_rel.py
@@ -1349,6 +1349,55 @@ class _PolymorphicTestBase(object):
             expected,
         )
 
+    def _join_to_poly_wp_one(self, sess):
+        wp = with_polymorphic(self.classes.Person, "*")
+        return (
+            sess.query(wp.name, self.classes.Company.name)
+            .join(self.classes.Company.employees.of_type(wp))
+            .order_by(wp.person_id)
+        )
+
+    def _join_to_poly_wp_two(self, sess):
+        wp = with_polymorphic(self.classes.Person, "*", aliased=True)
+        return (
+            sess.query(wp.name, self.classes.Company.name)
+            .join(self.classes.Company.employees.of_type(wp))
+            .order_by(wp.person_id)
+        )
+
+    def _join_to_poly_wp_three(self, sess):
+        wp = with_polymorphic(
+            self.classes.Person, "*", aliased=True, flat=True
+        )
+        return (
+            sess.query(wp.name, self.classes.Company.name)
+            .join(self.classes.Company.employees.of_type(wp))
+            .order_by(wp.person_id)
+        )
+
+    @testing.combinations(
+        lambda self, sess: (
+            sess.query(self.classes.Person.name, self.classes.Company.name)
+            .join(self.classes.Company.employees)
+            .order_by(self.classes.Person.person_id)
+        ),
+        _join_to_poly_wp_one,
+        _join_to_poly_wp_two,
+        _join_to_poly_wp_three,
+    )
+    def test_mixed_entities_join_to_poly(self, q):
+        sess = create_session()
+        expected = [
+            ("dilbert", "MegaCorp, Inc."),
+            ("wally", "MegaCorp, Inc."),
+            ("pointy haired boss", "MegaCorp, Inc."),
+            ("dogbert", "MegaCorp, Inc."),
+            ("vlad", "Elbonia, Inc."),
+        ]
+        eq_(
+            q(self, sess).all(), expected,
+        )
+
     def test_mixed_entities_two(self):
         sess = create_session()
         expected = [
-- 
2.39.5