]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Fixed bug when a query of the form:
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 9 Apr 2013 15:52:21 +0000 (11:52 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Tue, 9 Apr 2013 15:52:21 +0000 (11:52 -0400)
``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]

doc/build/changelog/changelog_08.rst
lib/sqlalchemy/orm/strategies.py
test/orm/inheritance/test_polymorphic_rel.py
test/orm/test_subquery_relations.py

index 4ef11590f9e3fd61815b53aa232ca1fd1b8e13ac..5046c6689955c9953e8e6823003cf1ab3f8d8d9f 100644 (file)
@@ -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
index 5c79de749fb419c0c9d7ceb579d43fac7bff635d..e08bb40cb729f79e92bae857b306265f2e5b0221 100644 (file)
@@ -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)
index e22848912f3450bcf2ed1f0d81835048c41fe227..1b9acb7871597967c026af9e303479e0b9e398f9 100644 (file)
@@ -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 = [
index a4cc830ee6ab627416bb3ee04b72916affbb9b6a..969d9f4c3f070b7a7c601b806472d0315aa72378 100644 (file)
@@ -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):