Compare mappers more accurately in Load._chop_path

Fixed bug in cache key generation for baked queries which could cause a
too-short cache key to be generated for the case of eager loads across
subclasses.  This could in turn cause the eagerload query to be cached in
place of a non-eagerload query, or vice versa, for a polymorhic "selectin"
load, or possibly for lazy loads or selectin loads as well.

Change-Id: I2a69349d3e38814e2c7e6012fc04fbc0e47658a4
Fixes: #4287

diff --git a/doc/build/changelog/unreleased_12/4287.rst b/doc/build/changelog/unreleased_12/4287.rst
new file mode 100644 (file)
index 0000000..d53327b
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4287.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4287
+
+    Fixed bug in cache key generation for baked queries which could cause a
+    too-short cache key to be generated for the case of eager loads across
+    subclasses.  This could in turn cause the eagerload query to be cached in
+    place of a non-eagerload query, or vice versa, for a polymorhic "selectin"
+    load, or possibly for lazy loads or selectin loads as well.

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index f54020fb74843a917c4ab49704f79c1e5d927b96..90d14075cb0e9641ea479aa802f39b54291195d5 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -385,6 +385,9 @@ class Load(Generative, MapperOption):
 
             if c_token is p_token:
                 continue
+            elif c_token.is_mapper and p_token.is_mapper and \
+                    c_token.isa(p_token):
+                continue
             else:
                 return None
         return to_chop[i + 1:]

diff --git a/test/orm/test_options.py b/test/orm/test_options.py
index 852ac66aac6d0c0afdbdfe6be9b8f912461c12d1..3c1f6527dbe05c0c4a8aa5fcc0f2cdc9e32f64d5 100644 (file)
--- a/test/orm/test_options.py
+++ b/test/orm/test_options.py
@@ -25,7 +25,14 @@ class QueryTest(_fixtures.FixtureTest):
         class SubItem(cls.classes.Item):
             pass
 
-        mapper(SubItem, None, inherits=cls.classes.Item)
+        mapper(
+            SubItem, None, inherits=cls.classes.Item,
+            properties={
+                "extra_keywords": relationship(
+                    cls.classes.Keyword, viewonly=True,
+                    secondary=cls.tables.item_keywords)
+            }
+        )
 
 
 class PathTest(object):
@@ -1221,6 +1228,40 @@ class CacheKeyTest(PathTest, QueryTest):
             None
         )
 
+    def test_unbound_cache_key_of_type_subclass_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 = subqueryload(
+            Order.items.of_type(SubItem)).subqueryload(SubItem.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")
+
+        query_path = self._make_path_registry([Order, "items", Item])
+
+        opt = Load(Order).subqueryload(
+            Order.items.of_type(SubItem)).subqueryload(SubItem.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')