From: Mike Bayer Date: Tue, 23 Mar 2021 02:56:36 +0000 (-0400) Subject: Remove internal use of string attr in loader option X-Git-Tag: rel_1_4_3~14^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9ec860e7a5c944799f9ef4de9069d707cfc1ed27;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Remove internal use of string attr in loader option Fixed issue where a "removed in 2.0" warning were generated internally by the relationship loader mechanics. This changeset started the effort of converting all string usage in the test suite, however this is a much longer job as the use of strings in loader options is widespread. In particular I'm not totally comfortable with strings not being accepted in obvious spots like Load(User).load_only("x", "y", "z"), which points to a new string expecting functionality that's not what's there now. However at the moment it seems like we need to continue removing all support for strings and then figure out "immediate strings from an explicit class" later. Fixes: #6115 Change-Id: I6b314d135d2bc049fd66500914b772c1fe60b5b3 --- diff --git a/doc/build/changelog/unreleased_14/6115.rst b/doc/build/changelog/unreleased_14/6115.rst new file mode 100644 index 0000000000..f866f9bce8 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6115.rst @@ -0,0 +1,7 @@ +.. change:: + :tags: bug, orm + :tickets: 6115 + + Fixed issue where a "removed in 2.0" warning were generated internally by + the relationship loader mechanics. + diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 7aae9ec37a..a660d7e1a5 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -83,6 +83,7 @@ class ORMFromClauseRole(roles.StrictFromClauseRole): _role_name = "ORM mapped entity, aliased entity, or FROM expression" +@inspection._self_inspects class MapperProperty( HasCacheKey, _MappedAttribute, InspectionAttr, util.MemoizedSlots ): diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 339a5bcf15..b11758090c 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -992,7 +992,7 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): compile_context.compile_options._current_path[ rev.parent ] - ).lazyload(rev.key).process_compile_state(compile_context) + ).lazyload(rev).process_compile_state(compile_context) stmt = stmt.add_criteria( lambda stmt: stmt._add_context_option( diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index 932c4a37d0..ba4e5c4664 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -234,84 +234,90 @@ class Load(Generative, LoaderOption): raise path = path[attr] - elif _is_mapped_class(attr): - # TODO: this does not appear to be a valid codepath. "attr" - # would never be a mapper. This block is present in 1.2 - # as well however does not seem to be accessed in any tests. - if not orm_util._entity_corresponds_to_use_path_impl( - attr.parent, path[-1] - ): - if raiseerr: - raise sa_exc.ArgumentError( - "Attribute '%s' does not " - "link from element '%s'" % (attr, path.entity) - ) - else: - return None else: - prop = found_property = attr.property - - if not orm_util._entity_corresponds_to_use_path_impl( - attr.parent, path[-1] - ): - if raiseerr: - raise sa_exc.ArgumentError( - 'Attribute "%s" does not ' - 'link from element "%s".%s' - % ( - attr, - path.entity, - ( - " Did you mean to use " - "%s.of_type(%s)?" - % (path[-2], attr.class_.__name__) - if len(path) > 1 - and path.entity.is_mapper - and attr.parent.is_aliased_class - else "" - ), + insp = inspect(attr) + + if insp.is_mapper or insp.is_aliased_class: + # TODO: this does not appear to be a valid codepath. "attr" + # would never be a mapper. This block is present in 1.2 + # as well however does not seem to be accessed in any tests. + if not orm_util._entity_corresponds_to_use_path_impl( + attr.parent, path[-1] + ): + if raiseerr: + raise sa_exc.ArgumentError( + "Attribute '%s' does not " + "link from element '%s'" % (attr, path.entity) ) - ) - else: - return None + else: + return None + elif insp.is_property: + prop = found_property = attr + path = path[prop] + elif insp.is_attribute: + prop = found_property = attr.property - if attr._extra_criteria: - self._extra_criteria = attr._extra_criteria + if not orm_util._entity_corresponds_to_use_path_impl( + attr.parent, path[-1] + ): + if raiseerr: + raise sa_exc.ArgumentError( + 'Attribute "%s" does not ' + 'link from element "%s".%s' + % ( + attr, + path.entity, + ( + " Did you mean to use " + "%s.of_type(%s)?" + % (path[-2], attr.class_.__name__) + if len(path) > 1 + and path.entity.is_mapper + and attr.parent.is_aliased_class + else "" + ), + ) + ) + else: + return None - if getattr(attr, "_of_type", None): - ac = attr._of_type - ext_info = of_type_info = inspect(ac) + if attr._extra_criteria: + self._extra_criteria = attr._extra_criteria - if polymorphic_entity_context is None: - polymorphic_entity_context = self.context + if getattr(attr, "_of_type", None): + ac = attr._of_type + ext_info = of_type_info = inspect(ac) - existing = path.entity_path[prop].get( - polymorphic_entity_context, "path_with_polymorphic" - ) + if polymorphic_entity_context is None: + polymorphic_entity_context = self.context - if not ext_info.is_aliased_class: - ac = orm_util.with_polymorphic( - ext_info.mapper.base_mapper, - ext_info.mapper, - aliased=True, - _use_mapper_path=True, - _existing_alias=inspect(existing) - if existing is not None - else None, + existing = path.entity_path[prop].get( + polymorphic_entity_context, "path_with_polymorphic" ) - ext_info = inspect(ac) + if not ext_info.is_aliased_class: + ac = orm_util.with_polymorphic( + ext_info.mapper.base_mapper, + ext_info.mapper, + aliased=True, + _use_mapper_path=True, + _existing_alias=inspect(existing) + if existing is not None + else None, + ) - path.entity_path[prop].set( - polymorphic_entity_context, "path_with_polymorphic", ac - ) + ext_info = inspect(ac) - path = path[prop][ext_info] + path.entity_path[prop].set( + polymorphic_entity_context, "path_with_polymorphic", ac + ) - self._of_type = of_type_info + path = path[prop][ext_info] - else: - path = path[prop] + self._of_type = of_type_info + + else: + path = path[prop] if for_strategy is not None: found_property._get_strategy(for_strategy) diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py index 289ac9a0a3..02137474b4 100644 --- a/lib/sqlalchemy/testing/assertions.py +++ b/lib/sqlalchemy/testing/assertions.py @@ -135,7 +135,12 @@ def uses_deprecated(*messages): @contextlib.contextmanager def _expect_warnings( - exc_cls, messages, regex=True, assert_=True, py2konly=False + exc_cls, + messages, + regex=True, + assert_=True, + py2konly=False, + raise_on_any_unexpected=False, ): if regex: @@ -145,7 +150,13 @@ def _expect_warnings( seen = set(filters) - real_warn = warnings.warn + if raise_on_any_unexpected: + + def real_warn(msg, *arg, **kw): + raise AssertionError("Got unexpected warning: %r" % msg) + + else: + real_warn = warnings.warn def our_warn(msg, *arg, **kw): if isinstance(msg, exc_cls): @@ -159,7 +170,7 @@ def _expect_warnings( if not exception or not issubclass(exception, exc_cls): return real_warn(msg, *arg, **kw) - if not filters: + if not filters and not raise_on_any_unexpected: return for filter_ in filters: diff --git a/test/orm/test_deprecations.py b/test/orm/test_deprecations.py index 34e3a48312..5c1349bd68 100644 --- a/test/orm/test_deprecations.py +++ b/test/orm/test_deprecations.py @@ -273,6 +273,29 @@ class DeprecatedQueryTest(_fixtures.FixtureTest, AssertsCompiledSQL): "ORDER BY addresses_1.id", ) + def test_dotted_options(self): + User = self.classes.User + + sess = fixture_session() + + with testing.expect_deprecated_20( + "Using strings to indicate column or relationship " + "paths in loader options" + ): + q2 = ( + sess.query(User) + .order_by(User.id) + .options(sa.orm.joinedload("orders")) + .options(sa.orm.joinedload("orders.items")) + .options(sa.orm.joinedload("orders.items.keywords")) + ) + u = q2.all() + + def go(): + u[0].orders[1].items[0].keywords[1] + + self.sql_count_(0, go) + def test_str_col_loader_opt(self): User = self.classes.User @@ -980,6 +1003,133 @@ class SelfRefFromSelfTest(fixtures.MappedTest, AssertsCompiledSQL): return testing.expect_deprecated_20(r"The Query.from_self\(\) method") +class SelfReferentialEagerTest(fixtures.MappedTest): + @classmethod + def define_tables(cls, metadata): + Table( + "nodes", + metadata, + Column( + "id", Integer, primary_key=True, test_needs_autoincrement=True + ), + Column("parent_id", Integer, ForeignKey("nodes.id")), + Column("data", String(30)), + ) + + def test_eager_loading_with_deferred(self): + nodes = self.tables.nodes + + class Node(fixtures.ComparableEntity): + def append(self, node): + self.children.append(node) + + mapper( + Node, + nodes, + properties={ + "children": relationship( + Node, lazy="joined", join_depth=3, order_by=nodes.c.id + ), + "data": deferred(nodes.c.data), + }, + ) + sess = fixture_session() + n1 = Node(data="n1") + n1.append(Node(data="n11")) + n1.append(Node(data="n12")) + sess.add(n1) + sess.flush() + sess.expunge_all() + + def go(): + with assertions.expect_deprecated_20( + "Using strings to indicate column or relationship paths" + ): + eq_( + Node( + data="n1", + children=[Node(data="n11"), Node(data="n12")], + ), + sess.query(Node) + .options(undefer("data"), undefer("children.data")) + .first(), + ) + + self.assert_sql_count(testing.db, go, 1) + + +class LazyLoadOptSpecificityTest(fixtures.DeclarativeMappedTest): + """test for [ticket:3963]""" + + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class A(Base): + __tablename__ = "a" + id = Column(Integer, primary_key=True) + bs = relationship("B") + + class B(Base): + __tablename__ = "b" + id = Column(Integer, primary_key=True) + a_id = Column(ForeignKey("a.id")) + cs = relationship("C") + + class C(Base): + __tablename__ = "c" + id = Column(Integer, primary_key=True) + b_id = Column(ForeignKey("b.id")) + + @classmethod + def insert_data(cls, connection): + A, B, C = cls.classes("A", "B", "C") + s = Session(connection) + s.add(A(id=1, bs=[B(cs=[C()])])) + s.add(A(id=2)) + s.commit() + + def _run_tests(self, query, expected): + def go(): + for a, _ in query: + for b in a.bs: + b.cs + + self.assert_sql_count(testing.db, go, expected) + + def test_string_options_aliased_whatever(self): + A, B, C = self.classes("A", "B", "C") + s = fixture_session() + aa = aliased(A) + q = ( + s.query(aa, A) + .filter(aa.id == 1) + .filter(A.id == 2) + .filter(aa.id != A.id) + .options(joinedload("bs").joinedload("cs")) + ) + with assertions.expect_deprecated_20( + "Using strings to indicate column or relationship paths" + ): + self._run_tests(q, 1) + + def test_string_options_unaliased_whatever(self): + A, B, C = self.classes("A", "B", "C") + s = fixture_session() + aa = aliased(A) + q = ( + s.query(A, aa) + .filter(aa.id == 2) + .filter(A.id == 1) + .filter(aa.id != A.id) + .options(joinedload("bs").joinedload("cs")) + ) + with assertions.expect_deprecated_20( + "Using strings to indicate column or relationship paths" + ): + self._run_tests(q, 1) + + class DynamicTest(_DynamicFixture, _fixtures.FixtureTest): def test_negative_slice_access_raises(self): User, Address = self._user_address_fixture() diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index 289f8e8b1e..b42c704310 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -109,7 +109,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): ) ], sess.query(User) - .options(joinedload("addresses")) + .options(joinedload(User.addresses)) .filter(User.id == 7) .all(), ) @@ -322,8 +322,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): sess = fixture_session() q = ( sess.query(User) - .join("addresses") - .options(joinedload("addresses")) + .join(User.addresses) + .options(joinedload(User.addresses)) .order_by("email_address") ) @@ -340,7 +340,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): q = ( sess.query(User) - .options(joinedload("addresses")) + .options(joinedload(User.addresses)) .order_by("email_address") ) @@ -573,7 +573,6 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): for opt, count in [ ((joinedload(User.orders, Order.items),), 10), - ((joinedload("orders.items"),), 10), ( ( joinedload(User.orders), @@ -700,8 +699,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): eq_( self.static.item_keyword_result[0:2], ( - q.options(joinedload("keywords")) - .join("keywords") + q.options(joinedload(Item.keywords)) + .join(Item.keywords) .filter(keywords.c.name == "red") ) .order_by(Item.id) @@ -1404,17 +1403,19 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): def go(): eq_(u1.addresses[0].user, u1) - self.assert_sql_execution( - testing.db, - go, - CompiledSQL( - "SELECT addresses.id AS addresses_id, addresses.user_id AS " - "addresses_user_id, addresses.email_address AS " - "addresses_email_address FROM addresses WHERE :param_1 = " - "addresses.user_id", - {"param_1": 8}, - ), - ) + with testing.expect_warnings(raise_on_any_unexpected=True): + self.assert_sql_execution( + testing.db, + go, + CompiledSQL( + "SELECT addresses.id AS addresses_id, " + "addresses.user_id AS " + "addresses_user_id, addresses.email_address AS " + "addresses_email_address FROM addresses WHERE :param_1 = " + "addresses.user_id", + {"param_1": 8}, + ), + ) def test_useget_cancels_eager_propagated_present(self): """test that a one to many lazyload cancels the unnecessary @@ -1453,17 +1454,19 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): def go(): eq_(u1.addresses[0].user, u1) - self.assert_sql_execution( - testing.db, - go, - CompiledSQL( - "SELECT addresses.id AS addresses_id, addresses.user_id AS " - "addresses_user_id, addresses.email_address AS " - "addresses_email_address FROM addresses WHERE :param_1 = " - "addresses.user_id", - {"param_1": 8}, - ), - ) + with testing.expect_warnings(raise_on_any_unexpected=True): + self.assert_sql_execution( + testing.db, + go, + CompiledSQL( + "SELECT addresses.id AS addresses_id, " + "addresses.user_id AS " + "addresses_user_id, addresses.email_address AS " + "addresses_email_address FROM addresses WHERE :param_1 = " + "addresses.user_id", + {"param_1": 8}, + ), + ) def test_manytoone_limit(self): """test that the subquery wrapping only occurs with @@ -1558,7 +1561,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): self.assert_compile( sess.query(User) - .options(joinedload("orders").joinedload("address")) + .options(joinedload(User.orders).joinedload(Order.address)) .limit(10), "SELECT anon_1.users_id AS anon_1_users_id, " "anon_1.users_name AS anon_1_users_name, " @@ -1580,8 +1583,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): self.assert_compile( sess.query(User).options( - joinedload("orders").joinedload("items"), - joinedload("orders").joinedload("address"), + joinedload(User.orders).joinedload(Order.items), + joinedload(User.orders).joinedload(Order.address), ), "SELECT users.id AS users_id, users.name AS users_name, " "items_1.id AS items_1_id, " @@ -1606,8 +1609,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): self.assert_compile( sess.query(User) .options( - joinedload("orders"), - joinedload("orders.address", innerjoin=True), + joinedload(User.orders), + joinedload(User.orders, Order.address, innerjoin=True), ) .limit(10), "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name " @@ -1630,8 +1633,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): self.assert_compile( sess.query(User) .options( - joinedload("orders", innerjoin=True), - joinedload("orders.address", innerjoin=True), + joinedload(User.orders, innerjoin=True), + joinedload(User.orders, Order.address, innerjoin=True), ) .limit(10), "SELECT anon_1.users_id AS anon_1_users_id, " @@ -1771,7 +1774,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): def go(): o1 = ( sess.query(Order) - .options(lazyload("address")) + .options(lazyload(Order.address)) .filter(Order.id == 5) .one() ) @@ -2317,8 +2320,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): sess = fixture_session() q = sess.query(User).options( - joinedload("orders", innerjoin=False).joinedload( - "items", innerjoin=True + joinedload(User.orders, innerjoin=False).joinedload( + Order.items, innerjoin=True ) ) @@ -2392,7 +2395,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): sess = fixture_session() q = sess.query(User).options( - joinedload("orders"), joinedload("addresses", innerjoin="unnested") + joinedload(User.orders), + joinedload(User.addresses, innerjoin="unnested"), ) self.assert_compile( @@ -2433,7 +2437,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): sess = fixture_session() q = sess.query(User).options( - joinedload("orders"), joinedload("addresses", innerjoin=True) + joinedload(User.orders), joinedload(User.addresses, innerjoin=True) ) self.assert_compile( @@ -2513,7 +2517,9 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): .join(User.orders) .join(Order.items) .options( - joinedload("orders").joinedload("items").joinedload("keywords") + joinedload(User.orders) + .joinedload(Order.items) + .joinedload(Item.keywords) ) ) @@ -2814,9 +2820,9 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): sess = fixture_session() if use_load: - opt = Load(User).defaultload("orders").lazyload("*") + opt = Load(User).defaultload(User.orders).lazyload("*") else: - opt = defaultload("orders").lazyload("*") + opt = defaultload(User.orders).lazyload("*") q = sess.query(User).filter(User.id == 7).options(opt) @@ -3180,15 +3186,15 @@ class InnerJoinSplicingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): def test_nested_innerjoin_propagation_multiple_paths_two(self): # test #3447 - A = self.classes.A + A, B, C1 = (self.classes.A, self.classes.B, self.classes.C1) s = fixture_session() q = s.query(A).options( - joinedload("bs"), - joinedload("bs.c2s", innerjoin=True), - joinedload("bs.c1s", innerjoin=True), - joinedload("bs.c1s.d1s"), + joinedload(A.bs), + joinedload(A.bs, B.c2s, innerjoin=True), + joinedload(A.bs, B.c1s, innerjoin=True), + joinedload(A.bs, B.c1s, C1.d1s), ) self.assert_compile( q, @@ -3208,17 +3214,23 @@ class InnerJoinSplicingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self._assert_result(q) def test_multiple_splice_points(self): - A = self.classes.A + A, B, C1, C2, D1 = ( + self.classes.A, + self.classes.B, + self.classes.C1, + self.classes.C2, + self.classes.D1, + ) s = fixture_session() q = s.query(A).options( - joinedload("bs", innerjoin=False), - joinedload("bs.c1s", innerjoin=True), - joinedload("bs.c2s", innerjoin=True), - joinedload("bs.c1s.d1s", innerjoin=False), - joinedload("bs.c2s.d2s"), - joinedload("bs.c1s.d1s.e1s", innerjoin=True), + joinedload(A.bs, innerjoin=False), + joinedload(A.bs, B.c1s, innerjoin=True), + joinedload(A.bs, B.c2s, innerjoin=True), + joinedload(A.bs, B.c1s, C1.d1s, innerjoin=False), + joinedload(A.bs, B.c2s, C2.d2s), + joinedload(A.bs, B.c1s, C1.d1s, D1.e1s, innerjoin=True), ) self.assert_compile( @@ -3260,7 +3272,7 @@ class InnerJoinSplicingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): s = fixture_session() - q = s.query(A).options(joinedload("bs_np", innerjoin=False)) + q = s.query(A).options(joinedload(A.bs_np, innerjoin=False)) self.assert_compile( q, "SELECT a.id AS a_id, b_1.id AS b_1_id, b_1.a_id AS b_1_a_id, " @@ -3372,13 +3384,13 @@ class InnerJoinSplicingWSecondaryTest( self.assert_sql_count(testing.db, go, 1) def test_joined_across(self): - A = self.classes.A + A, B, C = self.classes("A", "B", "C") s = fixture_session() q = s.query(A).options( - joinedload("b") - .joinedload("c", innerjoin=True) - .joinedload("ds", innerjoin=True) + joinedload(A.b) + .joinedload(B.c, innerjoin=True) + .joinedload(C.ds, innerjoin=True) ) self.assert_compile( q, @@ -3438,7 +3450,7 @@ class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self.assert_compile( fixture_session() .query(A) - .options(joinedload("bs")) + .options(joinedload(A.bs)) .order_by(A.summation) .limit(50), "SELECT anon_1.anon_2 AS anon_1_anon_2, anon_1.a_id " @@ -3461,7 +3473,7 @@ class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self.assert_compile( fixture_session() .query(A) - .options(joinedload("bs")) + .options(joinedload(A.bs)) .order_by(A.summation.desc()) .limit(50), "SELECT anon_1.anon_2 AS anon_1_anon_2, anon_1.a_id " @@ -3486,7 +3498,7 @@ class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self.assert_compile( fixture_session() .query(A) - .options(joinedload("bs")) + .options(joinedload(A.bs)) .order_by(A.summation) .limit(50), "SELECT anon_1.anon_2 AS anon_1_anon_2, anon_1.a_id " @@ -3515,7 +3527,7 @@ class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self.assert_compile( fixture_session() .query(A) - .options(joinedload("bs")) + .options(joinedload(A.bs)) .order_by(cp) .limit(50), "SELECT anon_1.a_id AS anon_1_a_id, anon_1.anon_2 " @@ -3542,7 +3554,7 @@ class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self.assert_compile( fixture_session() .query(A) - .options(joinedload("bs")) + .options(joinedload(A.bs)) .order_by(cp) .limit(50), "SELECT anon_1.a_id AS anon_1_a_id, anon_1.foo " @@ -3573,7 +3585,7 @@ class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): self.assert_compile( fixture_session() .query(A) - .options(joinedload("bs")) + .options(joinedload(A.bs)) .order_by(~cp) .limit(50), "SELECT anon_1.a_id AS anon_1_a_id, anon_1.anon_2 " @@ -3698,7 +3710,7 @@ class LoadOnExistingTest(_fixtures.FixtureTest): a2 = u1.addresses[0] a2.email_address = "foo" sess.query(User).options( - joinedload("addresses").joinedload("dingaling") + joinedload(User.addresses).joinedload(Address.dingaling) ).filter_by(id=8).all() assert u1.addresses[-1] is a1 for a in u1.addresses: @@ -3717,7 +3729,7 @@ class LoadOnExistingTest(_fixtures.FixtureTest): o1 = Order() u1.orders.append(o1) sess.query(User).options( - joinedload("orders").joinedload("items") + joinedload(User.orders).joinedload(Order.items) ).filter_by(id=7).all() for o in u1.orders: if o is not o1: @@ -3731,11 +3743,11 @@ class LoadOnExistingTest(_fixtures.FixtureTest): u1 = ( sess.query(User) .filter_by(id=8) - .options(joinedload("addresses")) + .options(joinedload(User.addresses)) .one() ) sess.query(User).filter_by(id=8).options( - joinedload("addresses").joinedload("dingaling") + joinedload(User.addresses).joinedload(Address.dingaling) ).first() assert "dingaling" in u1.addresses[0].__dict__ @@ -3745,11 +3757,11 @@ class LoadOnExistingTest(_fixtures.FixtureTest): u1 = ( sess.query(User) .filter_by(id=7) - .options(joinedload("orders")) + .options(joinedload(User.orders)) .one() ) sess.query(User).filter_by(id=7).options( - joinedload("orders").joinedload("items") + joinedload(User.orders).joinedload(Order.items) ).first() assert "items" in u1.orders[0].__dict__ @@ -3899,7 +3911,7 @@ class AddEntityTest(_fixtures.FixtureTest): def go(): ret = ( sess.query(User, oalias) - .options(joinedload("addresses")) + .options(joinedload(User.addresses)) .join(oalias, "orders") .order_by(User.id, oalias.id) .all() @@ -3913,7 +3925,7 @@ class AddEntityTest(_fixtures.FixtureTest): def go(): ret = ( sess.query(User, oalias) - .options(joinedload("addresses"), joinedload(oalias.items)) + .options(joinedload(User.addresses), joinedload(oalias.items)) .join(oalias, "orders") .order_by(User.id, oalias.id) .all() @@ -4174,7 +4186,7 @@ class SelfReferentialEagerTest(fixtures.MappedTest): eq_( Node(data="n1", children=[Node(data="n11"), Node(data="n12")]), sess.query(Node) - .options(undefer("data")) + .options(undefer(Node.data)) .order_by(Node.id) .first(), ) @@ -4187,7 +4199,10 @@ class SelfReferentialEagerTest(fixtures.MappedTest): eq_( Node(data="n1", children=[Node(data="n11"), Node(data="n12")]), sess.query(Node) - .options(undefer("data"), undefer("children.data")) + .options( + undefer(Node.data), + defaultload(Node.children).undefer(Node.data), + ) .first(), ) @@ -4226,7 +4241,7 @@ class SelfReferentialEagerTest(fixtures.MappedTest): sess.query(Node) .filter_by(data="n1") .order_by(Node.id) - .options(joinedload("children.children")) + .options(joinedload(Node.children, Node.children)) .first() ) eq_( @@ -4252,7 +4267,7 @@ class SelfReferentialEagerTest(fixtures.MappedTest): def go(): sess.query(Node).order_by(Node.id).filter_by(data="n1").options( - joinedload("children.children") + joinedload(Node.children, Node.children) ).first() # test that the query isn't wrapping the initial query for eager @@ -4409,9 +4424,9 @@ class MixedSelfReferentialEagerTest(fixtures.MappedTest): eq_( session.query(B) .options( - joinedload("parent_b1"), - joinedload("parent_b2"), - joinedload("parent_z"), + joinedload(B.parent_b1), + joinedload(B.parent_b2), + joinedload(B.parent_z), ) .filter(B.id.in_([2, 8, 11])) .order_by(B.id) @@ -5129,7 +5144,7 @@ class CorrelatedSubqueryTest(fixtures.MappedTest): eq_( sess.query(User) .order_by(User.name) - .options(joinedload("stuff")) + .options(joinedload(User.stuff)) .all(), [ User(name="user1", stuff=[Stuff(id=2)]), @@ -5156,7 +5171,7 @@ class CorrelatedSubqueryTest(fixtures.MappedTest): eq_( sess.query(User) .order_by(User.name) - .options(joinedload("stuff")) + .options(joinedload(User.stuff)) .first(), User(name="user1", stuff=[Stuff(id=2)]), ) @@ -5169,7 +5184,7 @@ class CorrelatedSubqueryTest(fixtures.MappedTest): eq_( sess.query(User) .filter(User.id == 2) - .options(joinedload("stuff")) + .options(joinedload(User.stuff)) .one(), User(name="user2", stuff=[Stuff(id=4)]), ) @@ -5413,7 +5428,7 @@ class EnsureColumnsAddedTest( self.assert_compile( s.query(Parent) - .options(load_only("data"), joinedload(Parent.o2mchild)) + .options(load_only(Parent.data), joinedload(Parent.o2mchild)) .limit(10), "SELECT anon_1.parent_id AS anon_1_parent_id, " "anon_1.parent_data AS anon_1_parent_data, " @@ -5433,7 +5448,7 @@ class EnsureColumnsAddedTest( self.assert_compile( s.query(Parent) - .options(load_only("data"), joinedload(Parent.m2mchild)) + .options(load_only(Parent.data), joinedload(Parent.m2mchild)) .limit(10), "SELECT anon_1.parent_id AS anon_1_parent_id, " "anon_1.parent_data AS anon_1_parent_data, " @@ -5455,7 +5470,7 @@ class EnsureColumnsAddedTest( self.assert_compile( s.query(Parent).options( - load_only("data"), joinedload(Parent.o2mchild) + load_only(Parent.data), joinedload(Parent.o2mchild) ), "SELECT parent.id AS parent_id, parent.data AS parent_data, " "parent.arb AS parent_arb, o2mchild_1.id AS o2mchild_1_id, " @@ -5471,7 +5486,7 @@ class EnsureColumnsAddedTest( self.assert_compile( s.query(Parent).options( - load_only("data"), joinedload(Parent.m2mchild) + load_only(Parent.data), joinedload(Parent.m2mchild) ), "SELECT parent.id AS parent_id, parent.data AS parent_data, " "parent.arb AS parent_arb, m2mchild_1.id AS m2mchild_1_id " @@ -5643,13 +5658,13 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest): # these paths don't work out correctly? lz_test = ( s.query(LDA) - .join("ld") - .options(contains_eager("ld")) + .join(LDA.ld) + .options(contains_eager(LDA.ld)) .join("a", (l_ac, "ld"), (u_ac, "user")) .options( - contains_eager("a") - .contains_eager("ld", alias=l_ac) - .contains_eager("user", alias=u_ac) + contains_eager(LDA.a) + .contains_eager(A.ld, alias=l_ac) + .contains_eager(LD.user, alias=u_ac) ) .first() ) @@ -5729,32 +5744,6 @@ class LazyLoadOptSpecificityTest(fixtures.DeclarativeMappedTest): self.assert_sql_count(testing.db, go, expected) - def test_string_options_aliased_whatever(self): - A, B, C = self.classes("A", "B", "C") - s = fixture_session() - aa = aliased(A) - q = ( - s.query(aa, A) - .filter(aa.id == 1) - .filter(A.id == 2) - .filter(aa.id != A.id) - .options(joinedload("bs").joinedload("cs")) - ) - self._run_tests(q, 1) - - def test_string_options_unaliased_whatever(self): - A, B, C = self.classes("A", "B", "C") - s = fixture_session() - aa = aliased(A) - q = ( - s.query(A, aa) - .filter(aa.id == 2) - .filter(A.id == 1) - .filter(aa.id != A.id) - .options(joinedload("bs").joinedload("cs")) - ) - self._run_tests(q, 1) - def test_lazyload_aliased_abs_bcs_one(self): A, B, C = self.classes("A", "B", "C") s = fixture_session() @@ -6034,7 +6023,7 @@ class DeepOptionsTest(_fixtures.FixtureTest): def test_deep_options_2(self): """test (joined|subquery)load_all() options""" - User = self.classes.User + User, Order, Item = self.classes("User", "Order", "Item") sess = fixture_session() @@ -6042,9 +6031,9 @@ class DeepOptionsTest(_fixtures.FixtureTest): sess.query(User) .order_by(User.id) .options( - sa.orm.joinedload("orders") - .joinedload("items") - .joinedload("keywords") + sa.orm.joinedload(User.orders) + .joinedload(Order.items) + .joinedload(Item.keywords) ) ).all() @@ -6057,9 +6046,9 @@ class DeepOptionsTest(_fixtures.FixtureTest): result = ( sess.query(User).options( - sa.orm.subqueryload("orders") - .subqueryload("items") - .subqueryload("keywords") + sa.orm.subqueryload(User.orders) + .subqueryload(Order.items) + .subqueryload(Item.keywords) ) ).all() @@ -6068,26 +6057,6 @@ class DeepOptionsTest(_fixtures.FixtureTest): self.sql_count_(0, go) - def test_deep_options_3(self): - User = self.classes.User - - sess = fixture_session() - - # same thing, with separate options calls - q2 = ( - sess.query(User) - .order_by(User.id) - .options(sa.orm.joinedload("orders")) - .options(sa.orm.joinedload("orders.items")) - .options(sa.orm.joinedload("orders.items.keywords")) - ) - u = q2.all() - - def go(): - u[0].orders[1].items[0].keywords[1] - - self.sql_count_(0, go) - def test_deep_options_4(self): Item, User, Order = ( self.classes.Item, @@ -6114,7 +6083,11 @@ class DeepOptionsTest(_fixtures.FixtureTest): q3 = ( sess.query(User) .order_by(User.id) - .options(sa.orm.joinedload("orders.items.keywords")) + .options( + sa.orm.defaultload(User.orders) + .defaultload(Order.items) + .joinedload(Item.keywords) + ) ) u = q3.all() diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py index 1cbe106062..47b7a8ff86 100644 --- a/test/orm/test_lazy_relations.py +++ b/test/orm/test_lazy_relations.py @@ -1516,7 +1516,7 @@ class RefersToSelfLazyLoadInterferenceTest(fixtures.MappedTest): # If the bug is here, the next line throws an exception session.query(B).options( - sa.orm.joinedload("parent").joinedload("zc") + sa.orm.joinedload(B.parent).joinedload(B.zc) ).all() diff --git a/test/orm/test_options.py b/test/orm/test_options.py index 6f47c1238a..4bef121d91 100644 --- a/test/orm/test_options.py +++ b/test/orm/test_options.py @@ -34,6 +34,10 @@ from .inheritance._poly_fixtures import Manager from .inheritance._poly_fixtures import Person +def _deprecated_strings(): + return testing.expect_deprecated_20("Using strings to indicate") + + class QueryTest(_fixtures.FixtureTest): run_setup_mappers = "once" run_inserts = "once" @@ -150,23 +154,28 @@ class LoadTest(PathTest, QueryTest): Address = self.classes.Address result = Load(User) - eq_( - result._generate_path( - inspect(User)._path_registry, "addresses", None, "relationship" - ), - self._make_path_registry([User, "addresses", Address]), - ) + with _deprecated_strings(): + eq_( + result._generate_path( + inspect(User)._path_registry, + "addresses", + None, + "relationship", + ), + self._make_path_registry([User, "addresses", Address]), + ) def test_gen_path_string_column(self): User = self.classes.User result = Load(User) - eq_( - result._generate_path( - inspect(User)._path_registry, "name", None, "column" - ), - self._make_path_registry([User, "name"]), - ) + with _deprecated_strings(): + eq_( + result._generate_path( + inspect(User)._path_registry, "name", None, "column" + ), + self._make_path_registry([User, "name"]), + ) def test_gen_path_invalid_from_col(self): User = self.classes.User @@ -207,13 +216,14 @@ class LoadTest(PathTest, QueryTest): sess = fixture_session() q = sess.query(OrderWProp).options(defer("some_attr")) - assert_raises_message( - sa.exc.ArgumentError, - r"Expected attribute \"some_attr\" on mapped class " - "OrderWProp->orders to be a mapped attribute; instead " - "got .*property.* object.", - q._compile_state, - ) + with _deprecated_strings(): + assert_raises_message( + sa.exc.ArgumentError, + r"Expected attribute \"some_attr\" on mapped class " + "OrderWProp->orders to be a mapped attribute; instead " + "got .*property.* object.", + q._compile_state, + ) def test_gen_path_attr_entity_invalid_noraiseerr(self): User = self.classes.User @@ -236,7 +246,7 @@ class LoadTest(PathTest, QueryTest): User = self.classes.User l1 = Load(User) - l2 = l1.joinedload("addresses") + l2 = l1.joinedload(User.addresses) to_bind = list(l2.context.values())[0] eq_( l1.context, @@ -247,7 +257,7 @@ class LoadTest(PathTest, QueryTest): User = self.classes.User l1 = Load(User) - l2 = l1.defer("name") + l2 = l1.defer(User.name) l3 = list(l2.context.values())[0] eq_(l1.context, {("loader", self._make_path([User, "name"])): l3}) @@ -1565,18 +1575,21 @@ class LocalOptsTest(PathTest, QueryTest): eq_(attr[key].local_opts, expected) def test_single_opt_only(self): + User = self.classes.User + opt = strategy_options._UnboundLoad().some_col_opt_only( - "name", {"foo": "bar"} + User.name, {"foo": "bar"} ) self._assert_attrs([opt], {"foo": "bar"}) def test_unbound_multiple_opt_only(self): + User = self.classes.User opts = [ strategy_options._UnboundLoad().some_col_opt_only( - "name", {"foo": "bar"} + User.name, {"foo": "bar"} ), strategy_options._UnboundLoad().some_col_opt_only( - "name", {"bat": "hoho"} + User.name, {"bat": "hoho"} ), ] self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) @@ -1585,8 +1598,8 @@ class LocalOptsTest(PathTest, QueryTest): User = self.classes.User opts = [ Load(User) - .some_col_opt_only("name", {"foo": "bar"}) - .some_col_opt_only("name", {"bat": "hoho"}) + .some_col_opt_only(User.name, {"foo": "bar"}) + .some_col_opt_only(User.name, {"bat": "hoho"}) ] self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) @@ -1594,29 +1607,31 @@ class LocalOptsTest(PathTest, QueryTest): User = self.classes.User opts = [ Load(User) - .some_col_opt_only("name", {"foo": "bar"}) - .some_col_opt_strategy("name", {"bat": "hoho"}) + .some_col_opt_only(User.name, {"foo": "bar"}) + .some_col_opt_strategy(User.name, {"bat": "hoho"}) ] self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) def test_unbound_strat_opt_recvs_from_optonly(self): + User = self.classes.User opts = [ strategy_options._UnboundLoad().some_col_opt_only( - "name", {"foo": "bar"} + User.name, {"foo": "bar"} ), strategy_options._UnboundLoad().some_col_opt_strategy( - "name", {"bat": "hoho"} + User.name, {"bat": "hoho"} ), ] self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) def test_unbound_opt_only_adds_to_strat(self): + User = self.classes.User opts = [ strategy_options._UnboundLoad().some_col_opt_strategy( - "name", {"bat": "hoho"} + User.name, {"bat": "hoho"} ), strategy_options._UnboundLoad().some_col_opt_only( - "name", {"foo": "bar"} + User.name, {"foo": "bar"} ), ] self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) @@ -1625,8 +1640,8 @@ class LocalOptsTest(PathTest, QueryTest): User = self.classes.User opts = [ Load(User) - .some_col_opt_strategy("name", {"bat": "hoho"}) - .some_col_opt_only("name", {"foo": "bar"}) + .some_col_opt_strategy(User.name, {"bat": "hoho"}) + .some_col_opt_only(User.name, {"foo": "bar"}) ] self._assert_attrs(opts, {"foo": "bar", "bat": "hoho"}) @@ -1937,7 +1952,7 @@ class MapperOptionsTest(_fixtures.FixtureTest): result = ( sess.query(User) .order_by(User.id) - .options(sa.orm.joinedload("addresses")) + .options(sa.orm.joinedload(User.addresses)) ).all() def go(): @@ -1966,7 +1981,7 @@ class MapperOptionsTest(_fixtures.FixtureTest): sess = fixture_session() u = ( sess.query(User) - .options(sa.orm.joinedload("addresses")) + .options(sa.orm.joinedload(User.addresses)) .filter_by(id=8) ).one() @@ -2003,7 +2018,7 @@ class MapperOptionsTest(_fixtures.FixtureTest): sess = fixture_session() u = ( sess.query(User) - .options(sa.orm.lazyload("addresses")) + .options(sa.orm.lazyload(User.addresses)) .filter_by(id=8) ).one() @@ -2184,7 +2199,7 @@ class MapperOptionsTest(_fixtures.FixtureTest): result = ( sess.query(User) .order_by(User.id) - .options(sa.orm.lazyload("addresses")) + .options(sa.orm.lazyload(User.addresses)) ).all() def go():