From 7d2a581a58e9ca4ffbcb39a384ba6950a966de7a Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 25 Jun 2018 00:23:54 -0400 Subject: [PATCH] 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 --- doc/build/changelog/unreleased_12/4287.rst | 9 +++++ lib/sqlalchemy/orm/strategy_options.py | 3 ++ test/orm/test_options.py | 43 +++++++++++++++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 doc/build/changelog/unreleased_12/4287.rst diff --git a/doc/build/changelog/unreleased_12/4287.rst b/doc/build/changelog/unreleased_12/4287.rst new file mode 100644 index 0000000000..d53327b5be --- /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 f54020fb74..90d14075cb 100644 --- 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 852ac66aac..3c1f6527db 100644 --- 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') -- 2.47.2