From: Mike Bayer Date: Tue, 9 Apr 2013 15:52:21 +0000 (-0400) Subject: Fixed bug when a query of the form: X-Git-Tag: rel_0_8_1~24^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=df4e59ff557b16202c4c3e47ad48667ba1e143c0;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_08.rst b/doc/build/changelog/changelog_08.rst index 4ef11590f9..5046c66899 100644 --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@ -6,6 +6,20 @@ .. changelog:: :version: 0.8.1 + .. 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 5c79de749f..e08bb40cb7 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -719,7 +719,7 @@ class SubqueryLoader(AbstractRelationshipLoader): # produce a subquery from it. left_alias = self._generate_from_original_query( orig_query, leftmost_mapper, - leftmost_attr + leftmost_attr, entity.mapper ) # generate another Query that will join the @@ -772,7 +772,7 @@ class SubqueryLoader(AbstractRelationshipLoader): def _generate_from_original_query(self, orig_query, leftmost_mapper, - leftmost_attr + leftmost_attr, entity_mapper ): # reformat the original query # to look only for significant columns @@ -781,6 +781,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) q._set_entities(q._adapt_col_list(leftmost_attr)) if q._order_by is False: @@ -792,6 +794,7 @@ class SubqueryLoader(AbstractRelationshipLoader): # the original query now becomes a subquery # which we'll join onto. + embed_q = q.with_labels().subquery() left_alias = orm_util.AliasedClass(leftmost_mapper, embed_q, use_mapper_path=True) diff --git a/test/orm/inheritance/test_polymorphic_rel.py b/test/orm/inheritance/test_polymorphic_rel.py index e22848912f..1b9acb7871 100644 --- a/test/orm/inheritance/test_polymorphic_rel.py +++ b/test/orm/inheritance/test_polymorphic_rel.py @@ -650,6 +650,7 @@ class _PolymorphicTestBase(object): count = 5 self.assert_sql_count(testing.db, go, count) + def test_joinedload_on_subclass(self): sess = create_session() expected = [ diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py index a4cc830ee6..969d9f4c3f 100644 --- a/test/orm/test_subquery_relations.py +++ b/test/orm/test_subquery_relations.py @@ -976,6 +976,115 @@ class OrderBySecondaryTest(fixtures.MappedTest): ]) self.assert_sql_count(testing.db, go, 2) + +from .inheritance._poly_fixtures import _Polymorphic, Person, Engineer, Paperwork + +class BaseRelationFromJoinedSubclassTest(_Polymorphic): + @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):