Set complete FROM list for subquery eagerload's orig query

Instead of checking that the "orig_entity" we receive applies
as a correct FROM element for the subquery we're building,
set the FROM clause of the query to exactly what it already
is based on column_descriptions (assuming there is no FROM
list already), thereby ensuring that the FROM list will remain
intact, regardless of what orig_entity turns out to be and
what the target_cols ultimately refer towards.

Fixed issue with subquery eagerloading which continues on from
the series of issues fixed in :ticket:`2699`, :ticket:`3106`,
:ticket:`3893` involving that the "subquery" contains the correct
FROM clause when beginning from a joined inheritance subclass
and then subquery eager loading onto a relationship from
the base class, while the query also includes criteria against
the subclass. The fix in the previous tickets did not accommodate
for additional subqueryload operations loading more deeply from
the first level, so the fix has been further generalized.

Change-Id: Ic909590814f71e577d8266b1dbc4c393dc48e019
Fixes: #4011

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 071cf074e2669b9447823ef5eb2a41e76992b0cc..f111c930a987953c38bde70df63dd971aadb50cb 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -29,6 +29,21 @@
         Fixed AttributeError which would occur in :class:`.WithinGroup`
         construct during an iteration of the structure.
 
+    .. change:: 4011
+        :tags: bug, orm
+        :tickets: 4011
+        :versions: 1.2.0b1
+
+        Fixed issue with subquery eagerloading which continues on from
+        the series of issues fixed in :ticket:`2699`, :ticket:`3106`,
+        :ticket:`3893` involving that the "subquery" contains the correct
+        FROM clause when beginning from a joined inheritance subclass
+        and then subquery eager loading onto a relationship from
+        the base class, while the query also includes criteria against
+        the subclass. The fix in the previous tickets did not accommodate
+        for additional subqueryload operations loading more deeply from
+        the first level, so the fix has been further generalized.
+
     .. change:: 4005
         :tags: bug, postgresql
         :tickets: 4005

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index e48462d359b66abab89b140a2c1fe3ba941d7f50..690984a6a709ac8d593c3b7bbc68e7935ab689b0 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -926,16 +926,23 @@ class SubqueryLoader(AbstractRelationshipLoader):
         # to look only for significant columns
         q = orig_query._clone().correlate(None)
 
-        # set a real "from" if not present, as this is more
-        # accurate than just going off of the column expression
-        if not q._from_obj and orig_entity.is_mapper and \
-                orig_entity.mapper.isa(leftmost_mapper):
-            q._set_select_from([orig_entity], False)
-        target_cols = q._adapt_col_list(leftmost_attr)
+        # set the query's "FROM" list explicitly to what the
+        # FROM list would be in any case, as we will be limiting
+        # the columns in the SELECT list which may no longer include
+        # all entities mentioned in things like WHERE, JOIN, etc.
+        if not q._from_obj:
+            q._set_select_from(
+                list(set([
+                    ent['entity'] for ent in orig_query.column_descriptions
+                ])),
+                False
+            )
 
-        # select from the identity columns of the outer.  This will remove
+        # select from the identity columns of the outer (specifically, these
+        # are the 'local_cols' of the property).  This will remove
         # other columns from the query that might suggest the right entity
-        # which is why we try to _set_select_from above.
+        # which is why we do _set_select_from above.
+        target_cols = q._adapt_col_list(leftmost_attr)
         q._set_entities(target_cols)
 
         distinct_target_key = leftmost_relationship.distinct_target_key

diff --git a/test/orm/inheritance/_poly_fixtures.py b/test/orm/inheritance/_poly_fixtures.py
index 4c83a34fad536ed3a511f00a65e1e9ed89401380..79ff456e47ca8c99c3b5e972f1228bf6644c0fbc 100644 (file)
--- a/test/orm/inheritance/_poly_fixtures.py
+++ b/test/orm/inheritance/_poly_fixtures.py
@@ -44,6 +44,10 @@ class Paperwork(fixtures.ComparableEntity):
     pass
 
 
+class Page(fixtures.ComparableEntity):
+    pass
+
+
 class _PolymorphicFixtureBase(fixtures.MappedTest, AssertsCompiledSQL):
     run_inserts = 'once'
     run_setup_mappers = 'once'

diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index 52be4ee4053b2679b21d6ba1c3fc1e82fb3ed81c..7561c0f7cfba3299daa038c11f8363ff926382c5 100644 (file)
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -17,7 +17,7 @@ import sqlalchemy as sa
 from sqlalchemy.orm import with_polymorphic
 
 from .inheritance._poly_fixtures import _Polymorphic, Person, Engineer, \
-    Paperwork, Machine, MachineType, Company
+    Paperwork, Page, Machine, MachineType, Company
 
 
 class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
@@ -1064,11 +1064,20 @@ class BaseRelationFromJoinedSubclassTest(_Polymorphic):
                           Column('person_id', Integer,
                                  ForeignKey('people.person_id')))
 
+        pages = Table(
+            'pages', metadata,
+            Column('page_id',
+                   Integer, primary_key=True, test_needs_autoincrement=True),
+            Column('stuff', String(50)),
+            Column('paperwork_id', ForeignKey('paperwork.paperwork_id'))
+        )
+
     @classmethod
     def setup_mappers(cls):
         people = cls.tables.people
         engineers = cls.tables.engineers
         paperwork = cls.tables.paperwork
+        pages = cls.tables.pages
 
         mapper(Person, people,
                polymorphic_on=people.c.type,
@@ -1081,15 +1090,26 @@ class BaseRelationFromJoinedSubclassTest(_Polymorphic):
                inherits=Person,
                polymorphic_identity='engineer')
 
-        mapper(Paperwork, paperwork)
+        mapper(Paperwork, paperwork, properties={
+            'pages': relationship(Page, order_by=pages.c.page_id)
+        })
+
+        mapper(Page, pages)
 
     @classmethod
     def insert_data(cls):
 
         e1 = Engineer(primary_language="java")
         e2 = Engineer(primary_language="c++")
-        e1.paperwork = [Paperwork(description="tps report #1"),
-                        Paperwork(description="tps report #2")]
+        e1.paperwork = [Paperwork(description="tps report #1",
+                                  pages=[
+                                      Page(stuff='report1 page1'),
+                                      Page(stuff='report1 page2')
+                                  ]),
+                        Paperwork(description="tps report #2",
+                                  pages=[
+                                      Page(stuff='report2 page1'),
+                                      Page(stuff='report2 page2')])]
         e2.paperwork = [Paperwork(description="tps report #3")]
         sess = create_session()
         sess.add_all([e1, e2])
@@ -1194,6 +1214,69 @@ class BaseRelationFromJoinedSubclassTest(_Polymorphic):
             )
         )
 
+    def test_correct_subquery_multilevel(self):
+        sess = create_session()
+        # use Person.paperwork here just to give the least
+        # amount of context
+        q = sess.query(Engineer).\
+            filter(Engineer.primary_language == 'java').\
+            options(
+                subqueryload(Engineer.paperwork).subqueryload(Paperwork.pages))
+
+        def go():
+            eq_(q.one().paperwork,
+                [Paperwork(description="tps report #1",
+                           pages=[Page(stuff='report1 page1'),
+                                  Page(stuff='report1 page2')]),
+                 Paperwork(description="tps report #2",
+                           pages=[Page(stuff='report2 page1'),
+                                  Page(stuff='report2 page2')])],
+
+                )
+        self.assert_sql_execution(
+            testing.db,
+            go,
+            CompiledSQL(
+                "SELECT people.person_id AS people_person_id, "
+                "people.name AS people_name, people.type AS people_type, "
+                "engineers.engineer_id AS engineers_engineer_id, "
+                "engineers.primary_language AS engineers_primary_language "
+                "FROM people JOIN engineers "
+                "ON people.person_id = engineers.engineer_id "
+                "WHERE engineers.primary_language = :primary_language_1",
+                {"primary_language_1": "java"}
+            ),
+            CompiledSQL(
+                "SELECT paperwork.paperwork_id AS paperwork_paperwork_id, "
+                "paperwork.description AS paperwork_description, "
+                "paperwork.person_id AS paperwork_person_id, "
+                "anon_1.people_person_id AS anon_1_people_person_id "
+                "FROM (SELECT people.person_id AS people_person_id "
+                "FROM people JOIN engineers "
+                "ON people.person_id = engineers.engineer_id "
+                "WHERE engineers.primary_language = :primary_language_1) "
+                "AS anon_1 JOIN paperwork "
+                "ON anon_1.people_person_id = paperwork.person_id "
+                "ORDER BY anon_1.people_person_id, paperwork.paperwork_id",
+                {"primary_language_1": "java"}
+            ),
+            CompiledSQL(
+                "SELECT pages.page_id AS pages_page_id, "
+                "pages.stuff AS pages_stuff, "
+                "pages.paperwork_id AS pages_paperwork_id, "
+                "paperwork_1.paperwork_id AS paperwork_1_paperwork_id "
+                "FROM (SELECT people.person_id AS people_person_id "
+                "FROM people JOIN engineers ON people.person_id = "
+                "engineers.engineer_id "
+                "WHERE engineers.primary_language = :primary_language_1) "
+                "AS anon_1 JOIN paperwork AS paperwork_1 "
+                "ON anon_1.people_person_id = paperwork_1.person_id "
+                "JOIN pages ON paperwork_1.paperwork_id = pages.paperwork_id "
+                "ORDER BY paperwork_1.paperwork_id, pages.page_id",
+                {"primary_language_1": "java"}
+            )
+        )
+
     def test_correct_subquery_with_polymorphic_no_alias(self):
         # test #3106
         sess = create_session()