From: Mike Bayer Date: Tue, 9 Apr 2013 15:58:59 +0000 (-0400) Subject: Fixed bug when a query of the form: X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ad1cd2b4ba4247b33d930d9720b3470816de447;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fixed bug when a query of the form: ``query(SubClass).options(subqueryload(Baseclass.attrname))``, where ``SubClass`` is a joined inh of ``BaseClass``, would fail to apply the ``JOIN`` inside the subquery on the attribute load, producing a cartesian product. The populated results still tended to be correct as additional rows are just ignored, so this issue may be present as a performance degradation in applications that are otherwise working correctly. [ticket:2699] --- diff --git a/doc/build/changelog/changelog_07.rst b/doc/build/changelog/changelog_07.rst index f1b30a55a7..94a2980f92 100644 --- a/doc/build/changelog/changelog_07.rst +++ b/doc/build/changelog/changelog_07.rst @@ -6,6 +6,20 @@ .. changelog:: :version: 0.7.11 + .. change:: + :tags: bug, orm + :tickets: 2699 + + Fixed bug when a query of the form: + ``query(SubClass).options(subqueryload(Baseclass.attrname))``, + where ``SubClass`` is a joined inh of ``BaseClass``, + would fail to apply the ``JOIN`` inside the subquery + on the attribute load, producing a cartesian product. + The populated results still tended to be correct as additional + rows are just ignored, so this issue may be present as a + performance degradation in applications that are + otherwise working correctly. + .. change:: :tags: bug, orm :tickets: 2689 diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 2cde3f67e0..eece47207b 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -745,7 +745,8 @@ class SubqueryLoader(AbstractRelationshipLoader): # produce a subquery from it. left_alias = self._generate_from_original_query( orig_query, leftmost_mapper, - leftmost_attr, subq_path + leftmost_attr, subq_path, + entity.mapper ) # generate another Query that will join the @@ -795,7 +796,7 @@ class SubqueryLoader(AbstractRelationshipLoader): def _generate_from_original_query(self, orig_query, leftmost_mapper, - leftmost_attr, subq_path + leftmost_attr, subq_path, entity_mapper ): # reformat the original query # to look only for significant columns @@ -804,6 +805,8 @@ class SubqueryLoader(AbstractRelationshipLoader): # TODO: why does polymporphic etc. require hardcoding # into _adapt_col_list ? Does query.add_columns(...) work # with polymorphic loading ? + if entity_mapper.isa(leftmost_mapper): + q._set_select_from(entity_mapper.mapped_table) q._set_entities(q._adapt_col_list(leftmost_attr)) if q._order_by is False: diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py index 73e8b62181..5f1aa8be88 100644 --- a/test/orm/test_subquery_relations.py +++ b/test/orm/test_subquery_relations.py @@ -978,6 +978,114 @@ class OrderBySecondaryTest(fixtures.MappedTest): ]) self.assert_sql_count(testing.db, go, 2) +from .inheritance.test_polymorphic_rel import Person, Engineer, Paperwork + +class BaseRelationFromJoinedSubclassTest(fixtures.MappedTest): + @classmethod + def define_tables(cls, metadata): + people = Table('people', metadata, + Column('person_id', Integer, + primary_key=True, + test_needs_autoincrement=True), + Column('name', String(50)), + Column('type', String(30))) + + # to test fully, PK of engineers table must be + # named differently from that of people + engineers = Table('engineers', metadata, + Column('engineer_id', Integer, + ForeignKey('people.person_id'), + primary_key=True), + Column('primary_language', String(50))) + + paperwork = Table('paperwork', metadata, + Column('paperwork_id', Integer, + primary_key=True, + test_needs_autoincrement=True), + Column('description', String(50)), + Column('person_id', Integer, + ForeignKey('people.person_id'))) + + @classmethod + def setup_mappers(cls): + people = cls.tables.people + engineers = cls.tables.engineers + paperwork = cls.tables.paperwork + + mapper(Person, people, + polymorphic_on=people.c.type, + polymorphic_identity='person', + properties={ + 'paperwork': relationship( + Paperwork)}) + + mapper(Engineer, engineers, + inherits=Person, + polymorphic_identity='engineer') + + mapper(Paperwork, paperwork) + + @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")] + e2.paperwork = [Paperwork(description="tps report #3")] + sess = create_session() + sess.add_all([e1, e2]) + sess.flush() + + def test_correct_subquery(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(Person.paperwork)) + def go(): + eq_(q.all()[0].paperwork, + [Paperwork(description="tps report #1"), + Paperwork(description="tps report #2")], + + ) + 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"} + ), + # ensure we get "people JOIN engineer" here, even though + # primary key "people.person_id" is against "Person" + # *and* the path comes out as "Person.paperwork", still + # want to select from "Engineer" entity + 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", + {"primary_language_1": "java"} + ) + ) + + + class SelfReferentialTest(fixtures.MappedTest): @classmethod def define_tables(cls, metadata):