From: Mike Bayer Date: Fri, 7 Dec 2018 21:01:04 +0000 (-0500) Subject: Refer to existing of_type when resolving string attribute name X-Git-Tag: rel_1_3_0b2~68^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=099f3fd812ff4424f90f3c2b41ddce7049a54022;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Refer to existing of_type when resolving string attribute name Fixed bug where chaining of mapper options using :meth:`.RelationshipProperty.of_type` in conjunction with a chained option that refers to an attribute name by string only would fail to locate the attribute. Fixes: #4400 Change-Id: I01bf449ec4d8f56bb8c34e25153c1c9b31ff8012 --- diff --git a/doc/build/changelog/unreleased_12/4400.rst b/doc/build/changelog/unreleased_12/4400.rst new file mode 100644 index 0000000000..4a76fb53cd --- /dev/null +++ b/doc/build/changelog/unreleased_12/4400.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: bug, orm + :tickests: 4400 + + Fixed bug where chaining of mapper options using + :meth:`.RelationshipProperty.of_type` in conjunction with a chained option + that refers to an attribute name by string only would fail to locate the + attribute. diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index 0ef34b0b9b..f0d2091101 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -164,6 +164,7 @@ class Load(Generative, MapperOption): query._attributes.update(self.context) def _generate_path(self, path, attr, wildcard_key, raiseerr=True): + existing_of_type = self._of_type self._of_type = None if raiseerr and not path.has_entity: @@ -188,16 +189,20 @@ class Load(Generative, MapperOption): self.path = path return path + if existing_of_type: + ent = inspect(existing_of_type) + else: + ent = path.entity try: # use getattr on the class to work around # synonyms, hybrids, etc. - attr = getattr(path.entity.class_, attr) + attr = getattr(ent.class_, attr) except AttributeError: if raiseerr: raise sa_exc.ArgumentError( "Can't find property named '%s' on the " "mapped entity %s in this Query. " % ( - attr, path.entity) + attr, ent) ) else: return None diff --git a/test/orm/test_options.py b/test/orm/test_options.py index e94b9e6b63..4e6f2f91fd 100644 --- a/test/orm/test_options.py +++ b/test/orm/test_options.py @@ -55,10 +55,14 @@ class PathTest(object): q._attributes = q._attributes.copy() attr = {} - for val in opt._to_bind: - val._bind_loader( - [ent.entity_zero for ent in q._mapper_entities], - q._current_path, attr, False) + if isinstance(opt, strategy_options._UnboundLoad): + for val in opt._to_bind: + val._bind_loader( + [ent.entity_zero for ent in q._mapper_entities], + q._current_path, attr, False) + else: + opt._process(q, True) + attr = q._attributes assert_paths = [k[1] for k in attr] eq_( @@ -183,6 +187,127 @@ class LoadTest(PathTest, QueryTest): ) +class OfTypePathingTest(PathTest, QueryTest): + def _fixture(self): + User, Address = self.classes.User, self.classes.Address + Dingaling = self.classes.Dingaling + address_table = self.tables.addresses + + class SubAddr(Address): + pass + + mapper(SubAddr, inherits=Address, properties={ + "sub_attr": column_property(address_table.c.email_address), + "dings": relationship(Dingaling) + }) + + return User, Address, SubAddr + + def test_oftype_only_col_attr_unbound(self): + User, Address, SubAddr = self._fixture() + + l1 = defaultload( + User.addresses.of_type(SubAddr)).defer(SubAddr.sub_attr) + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'sub_attr')] + ) + + def test_oftype_only_col_attr_bound(self): + User, Address, SubAddr = self._fixture() + + l1 = Load(User).defaultload( + User.addresses.of_type(SubAddr)).defer(SubAddr.sub_attr) + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'sub_attr')] + ) + + def test_oftype_only_col_attr_string_unbound(self): + User, Address, SubAddr = self._fixture() + + l1 = defaultload( + User.addresses.of_type(SubAddr)).defer("sub_attr") + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'sub_attr')] + ) + + def test_oftype_only_col_attr_string_bound(self): + User, Address, SubAddr = self._fixture() + + l1 = Load(User).defaultload( + User.addresses.of_type(SubAddr)).defer("sub_attr") + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'sub_attr')] + ) + + def test_oftype_only_rel_attr_unbound(self): + User, Address, SubAddr = self._fixture() + + l1 = defaultload( + User.addresses.of_type(SubAddr)).joinedload(SubAddr.dings) + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'dings')] + ) + + def test_oftype_only_rel_attr_bound(self): + User, Address, SubAddr = self._fixture() + + l1 = Load(User).defaultload( + User.addresses.of_type(SubAddr)).joinedload(SubAddr.dings) + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'dings')] + ) + + def test_oftype_only_rel_attr_string_unbound(self): + User, Address, SubAddr = self._fixture() + + l1 = defaultload( + User.addresses.of_type(SubAddr)).joinedload("dings") + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'dings')] + ) + + def test_oftype_only_rel_attr_string_bound(self): + User, Address, SubAddr = self._fixture() + + l1 = Load(User).defaultload( + User.addresses.of_type(SubAddr)).defer("sub_attr") + + sess = Session() + q = sess.query(User) + self._assert_path_result( + l1, q, + [(User, 'addresses'), (User, 'addresses', SubAddr, 'sub_attr')] + ) + + class OptionsTest(PathTest, QueryTest): def _option_fixture(self, *arg): @@ -439,6 +564,26 @@ class OptionsTest(PathTest, QueryTest): (u_mapper, u_mapper.attrs.addresses, a_mapper, a_mapper.attrs.user) ]) + def test_of_type_string_attr(self): + User, Address = self.classes.User, self.classes.Address + + sess = Session() + + class SubAddr(Address): + pass + mapper(SubAddr, inherits=Address) + + q = sess.query(User) + opt = self._option_fixture( + User.addresses.of_type(SubAddr), "user") + + u_mapper = inspect(User) + a_mapper = inspect(Address) + self._assert_path_result(opt, q, [ + (u_mapper, u_mapper.attrs.addresses), + (u_mapper, u_mapper.attrs.addresses, a_mapper, a_mapper.attrs.user) + ]) + def test_of_type_plus_level(self): Dingaling, User, Address = (self.classes.Dingaling, self.classes.User, @@ -1295,6 +1440,23 @@ class CacheKeyTest(PathTest, QueryTest): ) ) + def test_unbound_cache_key_of_type_subclass_relationship_stringattr(self): + User, Address, Order, Item, SubItem, Keyword = self.classes( + 'User', 'Address', 'Order', 'Item', 'SubItem', "Keyword") + + query_path = self._make_path_registry([Order, "items", Item]) + + opt = subqueryload( + Order.items.of_type(SubItem)).subqueryload("extra_keywords") + + eq_( + opt._generate_cache_key(query_path), + ( + (SubItem, ('lazy', 'subquery')), + ('extra_keywords', Keyword, ('lazy', 'subquery')) + ) + ) + def test_bound_cache_key_of_type_subclass_relationship(self): User, Address, Order, Item, SubItem, Keyword = self.classes( 'User', 'Address', 'Order', 'Item', 'SubItem', "Keyword") @@ -1312,6 +1474,23 @@ class CacheKeyTest(PathTest, QueryTest): ) ) + def test_bound_cache_key_of_type_subclass_string_relationship(self): + User, Address, Order, Item, SubItem, Keyword = self.classes( + 'User', 'Address', 'Order', 'Item', 'SubItem', "Keyword") + + query_path = self._make_path_registry([Order, "items", Item]) + + opt = Load(Order).subqueryload( + Order.items.of_type(SubItem)).subqueryload("extra_keywords") + + eq_( + opt._generate_cache_key(query_path), + ( + (SubItem, ('lazy', 'subquery')), + ('extra_keywords', Keyword, ('lazy', 'subquery')) + ) + ) + def test_unbound_cache_key_excluded_of_type_safe(self): User, Address, Order, Item, SubItem = self.classes( 'User', 'Address', 'Order', 'Item', 'SubItem')