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 (file)
index 0000000..4a76fb5
--- /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 0ef34b0b9bc0fdda2ffa99e61fba30aa1af0388f..f0d20911018a33ee52ee57d5dc8b34679d091e0c 100644 (file)
--- 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 e94b9e6b63b470b4d2e020a7093f882a67c3fda5..4e6f2f91fd322979e0c2c71eef93465a572a996f 100644 (file)
--- 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')