- Fixed joinedload bug which would occur when a. the query includes

limit/offset criteria that forces a subquery b. the relationship
uses "secondary" c. the primaryjoin of the relationship refers to
a column that is either not part of the primary key, or is a PK
col in a joined-inheritance subclass table that is under a different
attribute name than the parent table's primary key column d. the
query defers the columns that are present in the primaryjoin, typically
via not being included in load_only(); the necessary column(s) would
not be present in the subquery and produce invalid SQL.
fixes #3592

3 files changed, 126 insertions, 3 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 40a251a22..7efa3939f 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -21,6 +21,21 @@
     .. change::
         :tags: bug, orm
         :versions: 1.1.0b1
+        :tickets: 3592
+
+        Fixed joinedload bug which would occur when a. the query includes
+        limit/offset criteria that forces a subquery b. the relationship
+        uses "secondary" c. the primaryjoin of the relationship refers to
+        a column that is either not part of the primary key, or is a PK
+        col in a joined-inheritance subclass table that is under a different
+        attribute name than the parent table's primary key column d. the
+        query defers the columns that are present in the primaryjoin, typically
+        via not being included in load_only(); the necessary column(s) would
+        not be present in the subquery and produce invalid SQL.
+
+    .. change::
+        :tags: bug, orm
+        :versions: 1.1.0b1
         :tickets: 2696
 
         A rare case which occurs when a :meth:`.Session.rollback` fails in the
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 67dac1ccc..7de470dd5 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1367,8 +1367,7 @@ class JoinedLoader(AbstractRelationshipLoader):
         # send a hint to the Query as to where it may "splice" this join
         eagerjoin.stop_on = entity.selectable
 
-        if self.parent_property.secondary is None and \
-                not parentmapper:
+        if not parentmapper:
             # for parentclause that is the non-eager end of the join,
             # ensure all the parent cols in the primaryjoin are actually
             # in the
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 084741b27..1c3b57690 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -5,7 +5,7 @@ import sqlalchemy as sa
 from sqlalchemy import testing
 from sqlalchemy.orm import joinedload, deferred, undefer, \
     joinedload_all, backref, Session,\
-    defaultload, Load
+    defaultload, Load, load_only
 from sqlalchemy import Integer, String, Date, ForeignKey, and_, select, \
     func, text
 from sqlalchemy.testing.schema import Table, Column
@@ -4069,3 +4069,112 @@ class CyclicalInheritingEagerTestThree(fixtures.DeclarativeMappedTest,
             "director_1.id = persistent_1.id) "
             "ON director.other_id = persistent_1.id"
         )
+
+
+class EnsureColumnsAddedTest(
+        fixtures.DeclarativeMappedTest, testing.AssertsCompiledSQL):
+    __dialect__ = 'default'
+    run_create_tables = None
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class Parent(Base):
+            __tablename__ = 'parent'
+            id = Column(Integer, primary_key=True,
+                        test_needs_autoincrement=True)
+            arb = Column(Integer, unique=True)
+            data = Column(Integer)
+            o2mchild = relationship("O2MChild")
+            m2mchild = relationship("M2MChild", secondary=Table(
+                'parent_to_m2m', Base.metadata,
+                Column('parent_id', ForeignKey('parent.arb')),
+                Column('child_id', ForeignKey('m2mchild.id'))
+            ))
+
+        class O2MChild(Base):
+            __tablename__ = 'o2mchild'
+            id = Column(Integer, primary_key=True,
+                        test_needs_autoincrement=True)
+            parent_id = Column(ForeignKey('parent.arb'))
+
+        class M2MChild(Base):
+            __tablename__ = 'm2mchild'
+            id = Column(Integer, primary_key=True,
+                        test_needs_autoincrement=True)
+
+    def test_joinedload_defered_pk_limit_o2m(self):
+        Parent = self.classes.Parent
+
+        s = Session()
+
+        self.assert_compile(
+            s.query(Parent).options(
+                load_only('data'),
+                joinedload(Parent.o2mchild)).limit(10),
+            "SELECT anon_1.parent_id AS anon_1_parent_id, "
+            "anon_1.parent_data AS anon_1_parent_data, "
+            "anon_1.parent_arb AS anon_1_parent_arb, "
+            "o2mchild_1.id AS o2mchild_1_id, "
+            "o2mchild_1.parent_id AS o2mchild_1_parent_id "
+            "FROM (SELECT parent.id AS parent_id, parent.data AS parent_data, "
+            "parent.arb AS parent_arb FROM parent LIMIT :param_1) AS anon_1 "
+            "LEFT OUTER JOIN o2mchild AS o2mchild_1 "
+            "ON anon_1.parent_arb = o2mchild_1.parent_id"
+        )
+
+    def test_joinedload_defered_pk_limit_m2m(self):
+        Parent = self.classes.Parent
+
+        s = Session()
+
+        self.assert_compile(
+            s.query(Parent).options(
+                load_only('data'),
+                joinedload(Parent.m2mchild)).limit(10),
+            "SELECT anon_1.parent_id AS anon_1_parent_id, "
+            "anon_1.parent_data AS anon_1_parent_data, "
+            "anon_1.parent_arb AS anon_1_parent_arb, "
+            "m2mchild_1.id AS m2mchild_1_id "
+            "FROM (SELECT parent.id AS parent_id, "
+            "parent.data AS parent_data, parent.arb AS parent_arb "
+            "FROM parent LIMIT :param_1) AS anon_1 "
+            "LEFT OUTER JOIN (parent_to_m2m AS parent_to_m2m_1 "
+            "JOIN m2mchild AS m2mchild_1 "
+            "ON m2mchild_1.id = parent_to_m2m_1.child_id) "
+            "ON anon_1.parent_arb = parent_to_m2m_1.parent_id"
+        )
+
+    def test_joinedload_defered_pk_o2m(self):
+        Parent = self.classes.Parent
+
+        s = Session()
+
+        self.assert_compile(
+            s.query(Parent).options(
+                load_only('data'),
+                joinedload(Parent.o2mchild)),
+            "SELECT parent.id AS parent_id, parent.data AS parent_data, "
+            "parent.arb AS parent_arb, o2mchild_1.id AS o2mchild_1_id, "
+            "o2mchild_1.parent_id AS o2mchild_1_parent_id "
+            "FROM parent LEFT OUTER JOIN o2mchild AS o2mchild_1 "
+            "ON parent.arb = o2mchild_1.parent_id"
+        )
+
+    def test_joinedload_defered_pk_m2m(self):
+        Parent = self.classes.Parent
+
+        s = Session()
+
+        self.assert_compile(
+            s.query(Parent).options(
+                load_only('data'),
+                joinedload(Parent.m2mchild)),
+            "SELECT parent.id AS parent_id, parent.data AS parent_data, "
+            "parent.arb AS parent_arb, m2mchild_1.id AS m2mchild_1_id "
+            "FROM parent LEFT OUTER JOIN (parent_to_m2m AS parent_to_m2m_1 "
+            "JOIN m2mchild AS m2mchild_1 "
+            "ON m2mchild_1.id = parent_to_m2m_1.child_id) "
+            "ON parent.arb = parent_to_m2m_1.parent_id"
+        )