From: Mike Bayer Date: Fri, 26 Feb 2016 04:22:30 +0000 (-0500) Subject: - An improvement to the workings of :meth:`.Query.correlate` such X-Git-Tag: rel_1_1_0b1~98^2~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f8914288f012c4ef635531f09a0e13bcacacdb2a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - An improvement to the workings of :meth:`.Query.correlate` such that when a "polymorphic" entity is used which represents a straight join of several tables, the statement will ensure that all the tables within the join are part of what's correlating. fixes #3662 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index ccf99ee988..e06ae6a602 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,19 @@ .. changelog:: :version: 1.1.0b1 + .. change:: + :tags: bug, orm + :tickets: 3662 + + An improvement to the workings of :meth:`.Query.correlate` such + that when a "polymorphic" entity is used which represents a straight + join of several tables, the statement will ensure that all the + tables within the join are part of what's correlating. + + .. seealso:: + + :ref:`change_3662` + .. change:: :tags: bug, orm :tickets: 3431 diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst index fb0b72de7b..6c6febd081 100644 --- a/doc/build/changelog/migration_11.rst +++ b/doc/build/changelog/migration_11.rst @@ -16,7 +16,7 @@ What's New in SQLAlchemy 1.1? some issues may be moved to later milestones in order to allow for a timely release. - Document last updated: Feburary 9, 2016 + Document last updated: Feburary 25, 2016 Introduction ============ @@ -463,6 +463,72 @@ would have to be compared during the merge. :ticket:`3601` +.. _change_3662: + +Improvements to the Query.correlate method with polymoprhic entities +-------------------------------------------------------------------- + +In recent SQLAlchemy versions, the SQL generated by many forms of +"polymorphic" queries has a more "flat" form than it used to, where +a JOIN of several tables is no longer bundled into a subquery unconditionally. +To accommodate this, the :meth:`.Query.correlate` method now extracts the +individual tables from such a polymorphic selectable and ensures that all +are part of the "correlate" for the subquery. Assuming the +``Person/Manager/Engineer->Company`` setup from the mapping documentation, +using with_polymorphic:: + + sess.query(Person.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == Person.company_id). + correlate(Person).as_scalar() == "Elbonia, Inc.") + +The above query now produces:: + + SELECT people.name AS people_name + FROM people + LEFT OUTER JOIN engineers ON people.person_id = engineers.person_id + LEFT OUTER JOIN managers ON people.person_id = managers.person_id + WHERE (SELECT companies.name + FROM companies + WHERE companies.company_id = people.company_id) = ? + +Before the fix, the call to ``correlate(Person)`` would inadvertently +attempt to correlate to the join of ``Person``, ``Engineer`` and ``Manager`` +as a single unit, so ``Person`` wouldn't be correlated:: + + -- old, incorrect query + SELECT people.name AS people_name + FROM people + LEFT OUTER JOIN engineers ON people.person_id = engineers.person_id + LEFT OUTER JOIN managers ON people.person_id = managers.person_id + WHERE (SELECT companies.name + FROM companies, people + WHERE companies.company_id = people.company_id) = ? + +Using correlated subqueries against polymorphic mappings still has some +unpolished edges. If for example ``Person`` is polymorphically linked +to a so-called "concrete polymorphic union" query, the above subquery +may not correctly refer to this subquery. In all cases, a way to refer +to the "polyorphic" entity fully is to create an :func:`.aliased` object +from it first:: + + # works with all SQLAlchemy versions and all types of polymorphic + # aliasing. + + paliased = aliased(Person) + sess.query(paliased.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == paliased.company_id). + correlate(paliased).as_scalar() == "Elbonia, Inc.") + +The :func:`.aliased` construct guarantees that the "polymorphic selectable" +is wrapped in a subquery. By referring to it explicitly in the correlated +subquery, the polymorphic form is correctly used. + +:ticket:`3662` + .. _change_3081: Stringify of Query will consult the Session for the correct dialect diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index ad7b9130bf..f68a309178 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -882,12 +882,15 @@ class Query(object): a subquery as returned by :meth:`.Query.subquery` is embedded in another :func:`~.expression.select` construct. - """ + """ - self._correlate = self._correlate.union( - _interpret_as_from(s) - if s is not None else None - for s in args) + for s in args: + if s is None: + self._correlate = self._correlate.union([None]) + else: + self._correlate = self._correlate.union( + sql_util.surface_selectables(_interpret_as_from(s)) + ) @_generative() def autoflush(self, setting): diff --git a/test/orm/inheritance/test_polymorphic_rel.py b/test/orm/inheritance/test_polymorphic_rel.py index 29fbcff853..c82c30d592 100644 --- a/test/orm/inheritance/test_polymorphic_rel.py +++ b/test/orm/inheritance/test_polymorphic_rel.py @@ -1,7 +1,7 @@ from sqlalchemy import func, desc from sqlalchemy.orm import interfaces, create_session, joinedload, joinedload_all, \ subqueryload, subqueryload_all, aliased,\ - class_mapper + class_mapper, with_polymorphic from sqlalchemy import exc as sa_exc from sqlalchemy import testing @@ -1250,6 +1250,44 @@ class _PolymorphicTestBase(object): assert row.name == 'dilbert' assert row.primary_language == 'java' + def test_correlation_one(self): + sess = create_session() + + # unfortunately this pattern can't yet work for PolymorphicAliased + # and PolymorphicUnions, because the subquery does not compile + # out including the polymorphic selectable; only if Person is in + # the query() list does that happen. + eq_(sess.query(Person.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == Person.company_id). + correlate(Person).as_scalar() == "Elbonia, Inc.").all(), + [(e3.name, )]) + + def test_correlation_two(self): + sess = create_session() + + paliased = aliased(Person) + + eq_(sess.query(paliased.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == paliased.company_id). + correlate(paliased).as_scalar() == "Elbonia, Inc.").all(), + [(e3.name, )]) + + def test_correlation_three(self): + sess = create_session() + + paliased = aliased(Person, flat=True) + + eq_(sess.query(paliased.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == paliased.company_id). + correlate(paliased).as_scalar() == "Elbonia, Inc.").all(), + [(e3.name, )]) + class PolymorphicTest(_PolymorphicTestBase, _Polymorphic): def test_join_to_subclass_four(self): sess = create_session() @@ -1266,6 +1304,31 @@ class PolymorphicTest(_PolymorphicTestBase, _Polymorphic): .filter(Machine.name.ilike("%ibm%")).all(), [e1, e3]) + def test_correlation_w_polymorphic(self): + + sess = create_session() + + p_poly = with_polymorphic(Person, '*') + + eq_(sess.query(p_poly.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == p_poly.company_id). + correlate(p_poly).as_scalar() == "Elbonia, Inc.").all(), + [(e3.name, )]) + + def test_correlation_w_polymorphic_flat(self): + + sess = create_session() + + p_poly = with_polymorphic(Person, '*', flat=True) + + eq_(sess.query(p_poly.name) + .filter( + sess.query(Company.name). + filter(Company.company_id == p_poly.company_id). + correlate(p_poly).as_scalar() == "Elbonia, Inc.").all(), + [(e3.name, )]) def test_join_to_subclass_ten(self): pass @@ -1377,10 +1440,16 @@ class PolymorphicPolymorphicTest(_PolymorphicTestBase, _PolymorphicPolymorphic): class PolymorphicUnionsTest(_PolymorphicTestBase, _PolymorphicUnions): - pass + + @testing.fails() + def test_correlation_one(self): + super(PolymorphicUnionsTest, self).test_correlation_one() + class PolymorphicAliasedJoinsTest(_PolymorphicTestBase, _PolymorphicAliasedJoins): - pass + @testing.fails() + def test_correlation_one(self): + super(PolymorphicAliasedJoinsTest, self).test_correlation_one() class PolymorphicJoinsTest(_PolymorphicTestBase, _PolymorphicJoins): pass