From: Mike Bayer Date: Tue, 14 Jan 2020 22:32:12 +0000 (-0500) Subject: Establish that contains_eager()->alias can be replaced by of_type X-Git-Tag: rel_1_4_0b1~558 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5a6643a95dd00ec653150335ae6bf03b445c9e5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Establish that contains_eager()->alias can be replaced by of_type One test in test_of_type was creating a cartesian product because contains_eager() was used with "alias" to refer to a with_polymorphic(), but the wp was not used with of_type(), so the pathing did not know that additional entities were present. while the docs indicate that of_type() should be used, there is no reason to use "alias" when you are using of_type(). Attempts to make this automatic don't work as the current usage contract with "alias" is that the contains_eager() chain can continue along in terms of the base entities, which is another example of the implicit swapping of entities for an aliased version of themselves that really should be entirely marked as deprecated throughout 1.4 and removed in 2.0. So instead, add test coverage for the of_type() versions of things and begin to make the case that we can remove "alias" entirely, where previously we thought we would only deprecate the string form. Fixes: #5096 Change-Id: Ia7b021c4044332ab3282267815f208da64410e95 --- diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index c7b059fda1..f75c7d3bac 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -435,7 +435,8 @@ class PropComparator(operators.ColumnOperators): return a.of_type(class_) def of_type(self, class_): - r"""Redefine this object in terms of a polymorphic subclass. + r"""Redefine this object in terms of a polymorphic subclass, + :func:`.with_polymorphic` construct, or :func:`.aliased` construct. Returns a new PropComparator from which further criterion can be evaluated. diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index b123ecc5dd..0c72f3b37d 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -1037,28 +1037,41 @@ def contains_eager(loadopt, attr, alias=None): ``User`` entity, and the returned ``Order`` objects would have the ``Order.user`` attribute pre-populated. - :func:`.contains_eager` also accepts an `alias` argument, which is the - string name of an alias, an :func:`~sqlalchemy.sql.expression.alias` - construct, or an :func:`~sqlalchemy.orm.aliased` construct. Use this when - the eagerly-loaded rows are to come from an aliased table:: + When making use of aliases with :func:`.contains_eager`, the path + should be specified using :meth:`.PropComparator.of_type`:: user_alias = aliased(User) sess.query(Order).\ join((user_alias, Order.user)).\ - options(contains_eager(Order.user, alias=user_alias)) + options(contains_eager(Order.user.of_type(user_alias))) - When using :func:`.contains_eager` in conjunction with inherited - subclasses, the :meth:`.RelationshipProperty.of_type` modifier should - also be used in order to set up the pathing properly:: + :meth:`.PropComparator.of_type` is also used to indicate a join + against specific subclasses of an inherting mapper, or + of a :func:`.with_polymorphic` construct:: + # employees of a particular subtype sess.query(Company).\ outerjoin(Company.employees.of_type(Manager)).\ options( contains_eager( Company.employees.of_type(Manager), - alias=Manager) + ) ) + # employees of a multiple subtypes + wp = with_polymorphic(Employee, [Manager, Engineer]) + sess.query(Company).\ + outerjoin(Company.employees.of_type(wp)).\ + options( + contains_eager( + Company.employees.of_type(wp), + ) + ) + + The :paramref:`.contains_eager.alias` parameter is used for a similar + purpose, however the :meth:`.PropComparator.of_type` approach should work + in all cases and is more effective and explicit. + .. seealso:: :ref:`loading_toplevel` @@ -1070,6 +1083,7 @@ def contains_eager(loadopt, attr, alias=None): if not isinstance(alias, str): info = inspect(alias) alias = info.selectable + else: util.warn_deprecated( "Passing a string name for the 'alias' argument to " diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index a33da3512a..659f6e103e 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -5399,6 +5399,35 @@ class EntityViaMultiplePathTestOne(fixtures.DeclarativeMappedTest): # PYTHONHASHSEED in_("d", a1.c.__dict__) + def test_multi_path_load_of_type(self): + A, B, C, D = self.classes("A", "B", "C", "D") + + s = Session() + + c = C(d=D()) + + s.add(A(b=B(c=c), c=c)) + s.commit() + + c_alias_1 = aliased(C) + c_alias_2 = aliased(C) + + q = s.query(A) + q = q.join(A.b).join(B.c.of_type(c_alias_1)).join(c_alias_1.d) + q = q.options( + contains_eager(A.b) + .contains_eager(B.c.of_type(c_alias_1)) + .contains_eager(c_alias_1.d) + ) + q = q.join(A.c.of_type(c_alias_2)) + q = q.options(contains_eager(A.c.of_type(c_alias_2))) + + a1 = q.all()[0] + + # ensure 'd' key was populated in dict. Varies based on + # PYTHONHASHSEED + in_("d", a1.c.__dict__) + class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest): """test for [ticket:3431]""" @@ -5457,6 +5486,7 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest): l_ac = aliased(LD) u_ac = aliased(User) + # these paths don't work out correctly? lz_test = ( s.query(LDA) .join("ld") @@ -5472,6 +5502,39 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest): in_("user", lz_test.a.ld.__dict__) + def test_multi_path_load_of_type(self): + User, LD, A, LDA = self.classes("User", "LD", "A", "LDA") + + s = Session() + + u0 = User(data=42) + l0 = LD(user=u0) + z0 = A(ld=l0) + lz0 = LDA(ld=l0, a=z0) + s.add_all([u0, l0, z0, lz0]) + s.commit() + + l_ac = aliased(LD) + u_ac = aliased(User) + + lz_test = ( + s.query(LDA) + .join(LDA.ld) + .options(contains_eager(LDA.ld)) + .join(LDA.a) + .join(LDA.ld.of_type(l_ac)) + .join(l_ac.user.of_type(u_ac)) + .options( + contains_eager(LDA.a), + contains_eager(LDA.ld.of_type(l_ac)).contains_eager( + l_ac.user.of_type(u_ac) + ), + ) + .first() + ) + + in_("user", lz_test.a.ld.__dict__) + class LazyLoadOptSpecificityTest(fixtures.DeclarativeMappedTest): """test for [ticket:3963]""" diff --git a/test/orm/test_froms.py b/test/orm/test_froms.py index b22bb1601a..f3fda579ae 100644 --- a/test/orm/test_froms.py +++ b/test/orm/test_froms.py @@ -1109,6 +1109,36 @@ class InstancesTest(QueryTest, AssertsCompiledSQL): self.assert_sql_count(testing.db, go, 1) + def test_contains_eager_multi_aliased_of_type(self): + # test newer style that does not use the alias parameter + Item, User, Order = ( + self.classes.Item, + self.classes.User, + self.classes.Order, + ) + + sess = create_session() + q = sess.query(User) + + # test using Aliased with more than one level deep + oalias = aliased(Order) + ialias = aliased(Item) + + def go(): + result = ( + q.options( + contains_eager(User.orders.of_type(oalias)).contains_eager( + oalias.items.of_type(ialias) + ) + ) + .outerjoin(User.orders.of_type(oalias)) + .outerjoin(oalias.items.of_type(ialias)) + .order_by(User.id, oalias.id, ialias.id) + ) + assert self.static.user_order_result == result.all() + + self.assert_sql_count(testing.db, go, 1) + def test_contains_eager_chaining(self): """test that contains_eager() 'chains' by default.""" diff --git a/test/orm/test_of_type.py b/test/orm/test_of_type.py index a431d231f4..d6cbcf1201 100644 --- a/test/orm/test_of_type.py +++ b/test/orm/test_of_type.py @@ -151,22 +151,41 @@ class _PolymorphicTestBase(object): self.assert_sql_count(testing.db, go, 1) - def test_with_polymorphic_join_exec_contains_eager_two(self): + @testing.combinations( + # this form is not expected to work in all cases, ultimately + # the "alias" parameter should be deprecated entirely + # lambda Company, wp: contains_eager(Company.employees, alias=wp), + lambda Company, wp: contains_eager(Company.employees.of_type(wp)), + lambda Company, wp: contains_eager( + Company.employees.of_type(wp), alias=wp + ), + ) + def test_with_polymorphic_join_exec_contains_eager_two( + self, contains_eager_option + ): sess = Session() + wp = with_polymorphic(Person, [Engineer, Manager], aliased=True) + contains_eager_option = testing.resolve_lambda( + contains_eager_option, Company=Company, wp=wp + ) + q = ( + sess.query(Company) + .join(Company.employees.of_type(wp)) + .order_by(Company.company_id, wp.person_id) + .options(contains_eager_option) + ) + def go(): - wp = with_polymorphic(Person, [Engineer, Manager], aliased=True) - eq_( - sess.query(Company) - .join(Company.employees.of_type(wp)) - .order_by(Company.company_id, wp.person_id) - .options(contains_eager(Company.employees, alias=wp)) - .all(), - [self.c1, self.c2], - ) + eq_(q.all(), [self.c1, self.c2]) self.assert_sql_count(testing.db, go, 1) + self.assert_compile( + q, + self._test_with_polymorphic_join_exec_contains_eager_two_result(), + ) + def test_with_polymorphic_any(self): sess = Session() wp = with_polymorphic(Person, [Engineer], aliased=True) @@ -269,6 +288,38 @@ class PolymorphicPolymorphicTest( "ON companies.company_id = people_1.company_id" ) + def _test_with_polymorphic_join_exec_contains_eager_two_result(self): + return ( + "SELECT anon_1.people_person_id AS anon_1_people_person_id, " + "anon_1.people_company_id AS anon_1_people_company_id, " + "anon_1.people_name AS anon_1_people_name, " + "anon_1.people_type AS anon_1_people_type, " + "anon_1.engineers_person_id AS anon_1_engineers_person_id, " + "anon_1.engineers_status AS anon_1_engineers_status, " + "anon_1.engineers_engineer_name " + "AS anon_1_engineers_engineer_name, " + "anon_1.engineers_primary_language " + "AS anon_1_engineers_primary_language, anon_1.managers_person_id " + "AS anon_1_managers_person_id, anon_1.managers_status " + "AS anon_1_managers_status, anon_1.managers_manager_name " + "AS anon_1_managers_manager_name, companies.company_id " + "AS companies_company_id, companies.name AS companies_name " + "FROM companies JOIN (SELECT people.person_id AS " + "people_person_id, people.company_id AS people_company_id, " + "people.name AS people_name, people.type AS people_type, " + "engineers.person_id AS engineers_person_id, engineers.status " + "AS engineers_status, engineers.engineer_name " + "AS engineers_engineer_name, engineers.primary_language " + "AS engineers_primary_language, managers.person_id " + "AS managers_person_id, managers.status AS managers_status, " + "managers.manager_name AS managers_manager_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) AS anon_1 " + "ON companies.company_id = anon_1.people_company_id " + "ORDER BY companies.company_id, anon_1.people_person_id" + ) + class PolymorphicUnionsTest(_PolymorphicTestBase, _PolymorphicUnions): def _polymorphic_join_target(self, cls): @@ -290,6 +341,34 @@ class PolymorphicUnionsTest(_PolymorphicTestBase, _PolymorphicUnions): "AS pjoin_1 ON companies.company_id = pjoin_1.company_id" ) + def _test_with_polymorphic_join_exec_contains_eager_two_result(self): + return ( + "SELECT pjoin_1.person_id AS pjoin_1_person_id, " + "pjoin_1.company_id AS pjoin_1_company_id, pjoin_1.name AS " + "pjoin_1_name, pjoin_1.type AS pjoin_1_type, pjoin_1.status " + "AS pjoin_1_status, pjoin_1.engineer_name AS " + "pjoin_1_engineer_name, pjoin_1.primary_language AS " + "pjoin_1_primary_language, pjoin_1.manager_name AS " + "pjoin_1_manager_name, companies.company_id AS " + "companies_company_id, companies.name AS companies_name " + "FROM companies JOIN (SELECT engineers.person_id AS " + "person_id, people.company_id AS company_id, people.name AS name, " + "people.type AS type, engineers.status AS status, " + "engineers.engineer_name AS engineer_name, " + "engineers.primary_language AS primary_language, " + "CAST(NULL AS VARCHAR(50)) AS manager_name FROM people " + "JOIN engineers ON people.person_id = engineers.person_id " + "UNION ALL SELECT managers.person_id AS person_id, " + "people.company_id AS company_id, people.name AS name, " + "people.type AS type, managers.status AS status, " + "CAST(NULL AS VARCHAR(50)) AS engineer_name, " + "CAST(NULL AS VARCHAR(50)) AS primary_language, " + "managers.manager_name AS manager_name FROM people " + "JOIN managers ON people.person_id = managers.person_id) AS " + "pjoin_1 ON companies.company_id = pjoin_1.company_id " + "ORDER BY companies.company_id, pjoin_1.person_id" + ) + class PolymorphicAliasedJoinsTest( _PolymorphicTestBase, _PolymorphicAliasedJoins @@ -313,6 +392,37 @@ class PolymorphicAliasedJoinsTest( "ON companies.company_id = pjoin_1.people_company_id" ) + def _test_with_polymorphic_join_exec_contains_eager_two_result(self): + return ( + "SELECT pjoin_1.people_person_id AS pjoin_1_people_person_id, " + "pjoin_1.people_company_id AS pjoin_1_people_company_id, " + "pjoin_1.people_name AS pjoin_1_people_name, pjoin_1.people_type " + "AS pjoin_1_people_type, pjoin_1.engineers_person_id AS " + "pjoin_1_engineers_person_id, pjoin_1.engineers_status AS " + "pjoin_1_engineers_status, pjoin_1.engineers_engineer_name " + "AS pjoin_1_engineers_engineer_name, " + "pjoin_1.engineers_primary_language AS " + "pjoin_1_engineers_primary_language, pjoin_1.managers_person_id " + "AS pjoin_1_managers_person_id, pjoin_1.managers_status " + "AS pjoin_1_managers_status, pjoin_1.managers_manager_name " + "AS pjoin_1_managers_manager_name, companies.company_id " + "AS companies_company_id, companies.name AS companies_name " + "FROM companies JOIN (SELECT people.person_id AS " + "people_person_id, people.company_id AS people_company_id, " + "people.name AS people_name, people.type AS people_type, " + "engineers.person_id AS engineers_person_id, engineers.status " + "AS engineers_status, engineers.engineer_name AS " + "engineers_engineer_name, engineers.primary_language " + "AS engineers_primary_language, managers.person_id AS " + "managers_person_id, managers.status AS managers_status, " + "managers.manager_name AS managers_manager_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) AS pjoin_1 " + "ON companies.company_id = pjoin_1.people_company_id " + "ORDER BY companies.company_id, pjoin_1.people_person_id" + ) + class PolymorphicJoinsTest(_PolymorphicTestBase, _PolymorphicJoins): def _polymorphic_join_target(self, cls): @@ -324,6 +434,38 @@ class PolymorphicJoinsTest(_PolymorphicTestBase, _PolymorphicJoins): "ON companies.company_id = people_1.company_id" ) + def _test_with_polymorphic_join_exec_contains_eager_two_result(self): + return ( + "SELECT anon_1.people_person_id AS anon_1_people_person_id, " + "anon_1.people_company_id AS anon_1_people_company_id, " + "anon_1.people_name AS anon_1_people_name, " + "anon_1.people_type AS anon_1_people_type, " + "anon_1.engineers_person_id AS anon_1_engineers_person_id, " + "anon_1.engineers_status AS anon_1_engineers_status, " + "anon_1.engineers_engineer_name " + "AS anon_1_engineers_engineer_name, " + "anon_1.engineers_primary_language " + "AS anon_1_engineers_primary_language, anon_1.managers_person_id " + "AS anon_1_managers_person_id, anon_1.managers_status " + "AS anon_1_managers_status, anon_1.managers_manager_name " + "AS anon_1_managers_manager_name, companies.company_id " + "AS companies_company_id, companies.name AS companies_name " + "FROM companies JOIN (SELECT people.person_id AS " + "people_person_id, people.company_id AS people_company_id, " + "people.name AS people_name, people.type AS people_type, " + "engineers.person_id AS engineers_person_id, engineers.status " + "AS engineers_status, engineers.engineer_name " + "AS engineers_engineer_name, engineers.primary_language " + "AS engineers_primary_language, managers.person_id " + "AS managers_person_id, managers.status AS managers_status, " + "managers.manager_name AS managers_manager_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) AS anon_1 " + "ON companies.company_id = anon_1.people_company_id " + "ORDER BY companies.company_id, anon_1.people_person_id" + ) + def test_joinedload_explicit_with_unaliased_poly_compile(self): sess = Session() target = with_polymorphic(Person, Engineer)