From a991eedcf559e6992ada92913a29ed7022d62caa Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 15 Jun 2017 13:12:16 -0400 Subject: [PATCH] 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 --- doc/build/changelog/changelog_11.rst | 15 +++++ lib/sqlalchemy/orm/strategies.py | 23 ++++--- test/orm/inheritance/_poly_fixtures.py | 4 ++ test/orm/test_subquery_relations.py | 91 ++++++++++++++++++++++++-- 4 files changed, 121 insertions(+), 12 deletions(-) diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 071cf074e2..f111c930a9 100644 --- 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 e48462d359..690984a6a7 100644 --- 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 4c83a34fad..79ff456e47 100644 --- 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 52be4ee405..7561c0f7cf 100644 --- 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() -- 2.39.5