- Fixed an obscure bug where the wrong results would be

fetched when joining/joinedloading across a many-to-many
relationship to a single-table-inheriting
subclass with a specific discriminator value, due to "secondary"
rows that would come back.  The "secondary" and right-side
tables are now inner joined inside of parenthesis for all
ORM joins on many-to-many relationships so that the left->right
join can accurately filtered.
[ticket:2369]

2 files changed, 134 insertions, 13 deletions

diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py
index f30a37941..20c6de284 100644
--- a/test/orm/inheritance/test_relationship.py
+++ b/test/orm/inheritance/test_relationship.py
@@ -448,6 +448,7 @@ class M2MFilterTest(fixtures.MappedTest):
             [Organization(name='org1')])
 
 class SelfReferentialM2MTest(fixtures.MappedTest, AssertsCompiledSQL):
+    __dialect__ = "default"
 
     @classmethod
     def define_tables(cls, metadata):
@@ -550,8 +551,7 @@ class SelfReferentialM2MTest(fixtures.MappedTest, AssertsCompiledSQL):
             "(parent AS parent_1 JOIN child1 AS child1_1 ON parent_1.id = child1_1.id) "
             "ON parent_1.id = secondary_2.right_id WHERE "
             "parent_1.id = secondary_1.right_id AND :param_1 = "
-            "secondary_1.left_id",
-            dialect=default.DefaultDialect()
+            "secondary_1.left_id"
         )
 
     def test_eager_join(self):
@@ -571,14 +571,14 @@ class SelfReferentialM2MTest(fixtures.MappedTest, AssertsCompiledSQL):
             "child2_1.id AS child2_1_id, parent_1.id AS "
             "parent_1_id, parent_1.cls AS parent_1_cls FROM "
             "(SELECT child1.id AS child1_id, parent.id AS parent_id, "
-            "parent.cls AS parent_cls FROM parent JOIN child1 ON parent.id = "
-            "child1.id LIMIT :param_1) AS anon_1 LEFT OUTER JOIN secondary "
-            "AS secondary_1 ON anon_1.parent_id = secondary_1.right_id LEFT "
-            "OUTER JOIN (parent AS parent_1 JOIN child2 AS child2_1 ON "
-            "parent_1.id = child2_1.id) ON parent_1.id = "
-            "secondary_1.left_id",
-            {'param_1':1},
-            dialect=default.DefaultDialect())
+            "parent.cls AS parent_cls "
+            "FROM parent JOIN child1 ON parent.id = child1.id "
+            "LIMIT :param_1) AS anon_1 LEFT OUTER JOIN "
+            "(secondary AS secondary_1 JOIN "
+            "(parent AS parent_1 JOIN child2 AS child2_1 "
+            "ON parent_1.id = child2_1.id) ON parent_1.id = secondary_1.left_id) "
+            "ON anon_1.parent_id = secondary_1.right_id",
+            {'param_1':1})
 
         # another way to check
         assert q.limit(1).with_labels().subquery().count().scalar() == 1
diff --git a/test/orm/inheritance/test_single.py b/test/orm/inheritance/test_single.py
index de6e55e95..434642ca1 100644
--- a/test/orm/inheritance/test_single.py
+++ b/test/orm/inheritance/test_single.py
@@ -4,7 +4,7 @@ from sqlalchemy.orm import *
 
 from sqlalchemy import testing
 from test.orm import _fixtures
-from sqlalchemy.testing import fixtures
+from sqlalchemy.testing import fixtures, AssertsCompiledSQL
 from sqlalchemy.testing.schema import Table, Column
 
 
@@ -418,7 +418,8 @@ class RelationshipToSingleTest(testing.AssertsCompiledSQL, fixtures.MappedTest):
 
 
     def test_relationship_to_subclass(self):
-        JuniorEngineer, Company, companies, Manager, Employee, employees, Engineer = (self.classes.JuniorEngineer,
+        JuniorEngineer, Company, companies, Manager, \
+                    Employee, employees, Engineer = (self.classes.JuniorEngineer,
                                 self.classes.Company,
                                 self.tables.companies,
                                 self.classes.Manager,
@@ -511,6 +512,125 @@ class RelationshipToSingleTest(testing.AssertsCompiledSQL, fixtures.MappedTest):
             )
         go()
 
+
+class ManyToManyToSingleTest(fixtures.MappedTest, AssertsCompiledSQL):
+    __dialect__ = 'default'
+
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('parent', metadata,
+                Column('id', Integer, primary_key=True,
+                            test_needs_autoincrement=True)
+            )
+        Table('m2m', metadata,
+                Column('parent_id', Integer,
+                            ForeignKey('parent.id'), primary_key=True),
+                Column('child_id', Integer,
+                            ForeignKey('child.id'), primary_key=True),
+            )
+        Table('child', metadata,
+                Column('id', Integer, primary_key=True,
+                            test_needs_autoincrement=True),
+                Column('discriminator', String(20)),
+                Column('name', String(20))
+            )
+
+    @classmethod
+    def setup_classes(cls):
+        class Parent(cls.Comparable):
+            pass
+
+        class Child(cls.Comparable):
+            pass
+
+        class SubChild1(Child):
+            pass
+
+        class SubChild2(Child):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        mapper(cls.classes.Parent, cls.tables.parent, properties={
+                "s1": relationship(cls.classes.SubChild1,
+                        secondary=cls.tables.m2m,
+                        uselist=False),
+                "s2": relationship(cls.classes.SubChild2,
+                        secondary=cls.tables.m2m)
+            })
+        mapper(cls.classes.Child, cls.tables.child,
+                        polymorphic_on=cls.tables.child.c.discriminator)
+        mapper(cls.classes.SubChild1, inherits=cls.classes.Child,
+                    polymorphic_identity='sub1')
+        mapper(cls.classes.SubChild2, inherits=cls.classes.Child,
+                    polymorphic_identity='sub2')
+
+    @classmethod
+    def insert_data(cls):
+        Parent = cls.classes.Parent
+        SubChild1 = cls.classes.SubChild1
+        SubChild2 = cls.classes.SubChild2
+        s = Session()
+        s.add_all([
+                Parent(s1=SubChild1(name='sc1_1'),
+                    s2=[SubChild2(name="sc2_1"), SubChild2(name="sc2_2")]
+                ),
+        ])
+        s.commit()
+
+    def test_eager_join(self):
+        Parent = self.classes.Parent
+        SubChild1 = self.classes.SubChild1
+
+        s = Session()
+
+        p1 = s.query(Parent).options(joinedload(Parent.s1)).all()[0]
+        eq_(p1.__dict__['s1'], SubChild1(name='sc1_1'))
+
+    def test_manual_join(self):
+        Parent = self.classes.Parent
+        Child = self.classes.Child
+        SubChild1 = self.classes.SubChild1
+
+        s = Session()
+
+        p1, c1 = s.query(Parent, Child).outerjoin(Parent.s1).all()[0]
+        eq_(c1, SubChild1(name='sc1_1'))
+
+    def test_assert_join_sql(self):
+        Parent = self.classes.Parent
+        Child = self.classes.Child
+
+        s = Session()
+
+        self.assert_compile(
+            s.query(Parent, Child).outerjoin(Parent.s1),
+            "SELECT parent.id AS parent_id, child.id AS child_id, "
+            "child.discriminator AS child_discriminator, "
+            "child.name AS child_name "
+            "FROM parent LEFT OUTER JOIN (m2m AS m2m_1 "
+            "JOIN child ON child.id = m2m_1.child_id "
+            "AND child.discriminator IN (:discriminator_1)) "
+            "ON parent.id = m2m_1.parent_id"
+        )
+
+    def test_assert_joinedload_sql(self):
+        Parent = self.classes.Parent
+        Child = self.classes.Child
+
+        s = Session()
+
+        self.assert_compile(
+            s.query(Parent).options(joinedload(Parent.s1)),
+            "SELECT parent.id AS parent_id, child_1.id AS child_1_id, "
+            "child_1.discriminator AS child_1_discriminator, "
+            "child_1.name AS child_1_name "
+            "FROM parent LEFT OUTER JOIN "
+            "(m2m AS m2m_1 JOIN child AS child_1 "
+            "ON child_1.id = m2m_1.child_id AND child_1.discriminator "
+            "IN (:discriminator_1)) ON parent.id = m2m_1.parent_id"
+        )
+
 class SingleOnJoinedTest(fixtures.MappedTest):
     @classmethod
     def define_tables(cls, metadata):
@@ -536,7 +656,8 @@ class SingleOnJoinedTest(fixtures.MappedTest):
         class Manager(Employee):
             pass
 
-        mapper(Person, persons_table, polymorphic_on=persons_table.c.type, polymorphic_identity='person')
+        mapper(Person, persons_table, polymorphic_on=persons_table.c.type,
+                    polymorphic_identity='person')
         mapper(Employee, employees_table, inherits=Person,polymorphic_identity='engineer')
         mapper(Manager, inherits=Employee,polymorphic_identity='manager')