- Fixed bug in single table inheritance where a chain of joins

that included the same single inh entity more than once
(normally this should raise an error) could, in some cases
depending on what was being joined "from", implicitly alias the
second case of the single inh entity, producing
a query that "worked".   But as this implicit aliasing is not
intended in the case of single table inheritance, it didn't
really "work" fully and was very misleading, since it wouldn't
always appear.
fixes #3233

4 files changed, 165 insertions, 5 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index f2b1b9a6c..0a6653883 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -155,6 +155,25 @@
 
     .. change::
         :tags: bug, orm
+        :tickets: 3233
+
+        Fixed bug in single table inheritance where a chain of joins
+        that included the same single inh entity more than once
+        (normally this should raise an error) could, in some cases
+        depending on what was being joined "from", implicitly alias the
+        second case of the single inh entity, producing
+        a query that "worked".   But as this implicit aliasing is not
+        intended in the case of single table inheritance, it didn't
+        really "work" fully and was very misleading, since it wouldn't
+        always appear.
+
+        .. seealso::
+
+            :ref:`bug_3233`
+
+
+    .. change::
+        :tags: bug, orm
         :tickets: 3222
 
         The ON clause rendered when using :meth:`.Query.join`,
diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst
index 65a8d4431..819acbe53 100644
--- a/doc/build/changelog/migration_10.rst
+++ b/doc/build/changelog/migration_10.rst
@@ -8,7 +8,7 @@ What's New in SQLAlchemy 1.0?
     undergoing maintenance releases as of May, 2014,
     and SQLAlchemy version 1.0, as of yet unreleased.
 
-    Document last updated: September 25, 2014
+    Document last updated: October 23, 2014
 
 Introduction
 ============
@@ -710,6 +710,90 @@ fine if the criteria happens to be rendered twice in the meantime.
 
 :ticket:`3222`
 
+.. _bug_3233:
+
+Single inheritance join targets will no longer sometimes implicitly alias themselves
+------------------------------------------------------------------------------------
+
+This is a bug where an unexpected and inconsistent behavior would occur
+in some scenarios when joining to a single-table-inheritance entity.  The
+difficulty this might cause is that the query is supposed to raise an error,
+as it is invalid SQL, however the bug would cause an alias to be added which
+makes the query "work".   The issue is confusing because this aliasing
+is not applied consistently and could change based on the nature of the query
+preceding the join.
+
+A simple example is::
+
+    from sqlalchemy import Integer, Column, String, ForeignKey
+    from sqlalchemy.orm import Session, relationship
+    from sqlalchemy.ext.declarative import declarative_base
+
+    Base = declarative_base()
+
+    class A(Base):
+        __tablename__ = "a"
+
+        id = Column(Integer, primary_key=True)
+        type = Column(String)
+
+        __mapper_args__ = {'polymorphic_on': type, 'polymorphic_identity': 'a'}
+
+
+    class ASub1(A):
+        __mapper_args__ = {'polymorphic_identity': 'asub1'}
+
+
+    class ASub2(A):
+        __mapper_args__ = {'polymorphic_identity': 'asub2'}
+
+
+    class B(Base):
+        __tablename__ = 'b'
+
+        id = Column(Integer, primary_key=True)
+
+        a_id = Column(Integer, ForeignKey("a.id"))
+
+        a = relationship("A", primaryjoin="B.a_id == A.id", backref='b')
+
+    s = Session()
+
+    print s.query(ASub1).join(B, ASub1.b).join(ASub2, B.a)
+
+    print s.query(ASub1).join(B, ASub1.b).join(ASub2, ASub2.id == B.a_id)
+
+The two queries at the bottom are equivalent, and should both render
+the identical SQL:
+
+    SELECT a.id AS a_id, a.type AS a_type
+    FROM a JOIN b ON b.a_id = a.id JOIN a ON b.a_id = a.id AND a.type IN (:type_1)
+    WHERE a.type IN (:type_2)
+
+The above SQL is invalid, as it renders "a" within the FROM list twice.
+The bug however would occur with the second query only and render this instead::
+
+    SELECT a.id AS a_id, a.type AS a_type
+    FROM a JOIN b ON b.a_id = a.id JOIN a AS a_1
+    ON a_1.id = b.a_id AND a_1.type IN (:type_1)
+    WHERE a_1.type IN (:type_2)
+
+Where above, the second join to "a" is aliased.  While this seems convenient,
+it's not how single-inheritance queries work in general and is misleading
+and inconsistent.
+
+The net effect is that applications which were relying on this bug will now
+have an error raised by the database.   The solution is to use the expected
+form.  When referring to multiple subclasses of a single-inheritance
+entity in a query, you must manually use aliases to disambiguate the table,
+as all the subclasses normally refer to the same table::
+
+    asub2_alias = aliased(ASub2)
+
+    print s.query(ASub1).join(B, ASub1.b).join(asub2_alias, B.a.of_type(asub2_alias))
+
+:ticket:`3233`
+
 .. _bug_3188:
 
 ColumnProperty constructs work a lot better with aliases, order_by
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index dc09e8eb4..f07060825 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -1947,11 +1947,9 @@ class Query(object):
                                 from_obj, r_info.selectable):
                         overlap = True
                         break
-            elif sql_util.selectables_overlap(l_info.selectable,
-                                              r_info.selectable):
-                overlap = True
 
-        if overlap and l_info.selectable is r_info.selectable:
+        if (overlap or not create_aliases) and \
+                l_info.selectable is r_info.selectable:
             raise sa_exc.InvalidRequestError(
                 "Can't join table/selectable '%s' to itself" %
                 l_info.selectable)
diff --git a/test/orm/inheritance/test_single.py b/test/orm/inheritance/test_single.py
index 967c07150..dbbe4c435 100644
--- a/test/orm/inheritance/test_single.py
+++ b/test/orm/inheritance/test_single.py
@@ -573,6 +573,65 @@ class RelationshipToSingleTest(testing.AssertsCompiledSQL, fixtures.MappedTest):
             "AND employees_1.type IN (:type_1)"
         )
 
+    def test_no_aliasing_from_overlap(self):
+        # test [ticket:3233]
+
+        Company, Employee, Engineer, Manager = self.classes.Company,\
+                                self.classes.Employee,\
+                                self.classes.Engineer,\
+                                self.classes.Manager
+
+        companies, employees = self.tables.companies, self.tables.employees
+
+        mapper(Company, companies, properties={
+            'employees': relationship(Employee, backref="company")
+        })
+        mapper(Employee, employees, polymorphic_on=employees.c.type)
+        mapper(Engineer, inherits=Employee, polymorphic_identity='engineer')
+        mapper(Manager, inherits=Employee, polymorphic_identity='manager')
+
+        s = create_session()
+
+        q1 = s.query(Engineer).\
+            join(Engineer.company).\
+            join(Manager, Company.employees)
+
+        q2 = s.query(Engineer).\
+            join(Engineer.company).\
+            join(Manager, Company.company_id == Manager.company_id)
+
+        q3 = s.query(Engineer).\
+            join(Engineer.company).\
+            join(Manager, Company.employees.of_type(Manager))
+
+        q4 = s.query(Engineer).\
+            join(Company, Company.company_id == Engineer.company_id).\
+            join(Manager, Company.employees.of_type(Manager))
+
+        q5 = s.query(Engineer).\
+            join(Company, Company.company_id == Engineer.company_id).\
+            join(Manager, Company.company_id == Manager.company_id)
+
+        # note that the query is incorrect SQL; we JOIN to
+        # employees twice.   However, this is what's expected so we seek
+        # to be consistent; previously, aliasing would sneak in due to the
+        # nature of the "left" side.
+        for q in [q1, q2, q3, q4, q5]:
+            self.assert_compile(
+                q,
+                "SELECT employees.employee_id AS employees_employee_id, "
+                "employees.name AS employees_name, "
+                "employees.manager_data AS employees_manager_data, "
+                "employees.engineer_info AS employees_engineer_info, "
+                "employees.type AS employees_type, "
+                "employees.company_id AS employees_company_id "
+                "FROM employees JOIN companies "
+                "ON companies.company_id = employees.company_id "
+                "JOIN employees "
+                "ON companies.company_id = employees.company_id "
+                "AND employees.type IN (:type_1) "
+                "WHERE employees.type IN (:type_2)"
+            )
 
     def test_relationship_to_subclass(self):
         JuniorEngineer, Company, companies, Manager, \