From a58f1b9c698dc7be29d43f2c4c21de8918943f77 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 17 Aug 2022 13:06:51 -0400 Subject: [PATCH] fill out all distinguising fields for AliasedInsp Hardened the cache key strategy for the :func:`_orm.aliased` and :func:`_orm.with_polymorphic` constructs. While no issue involving actual statements being cached can easily be demonstrated (if at all), these two constructs were not including enough of what makes them unique in their cache keys for caching on the aliased construct alone to be accurate. Fixes: #8401 Change-Id: I13d14985b6965f398edd9494601d8ae89ac641f2 --- doc/build/changelog/unreleased_14/8401.rst | 9 ++ lib/sqlalchemy/orm/util.py | 19 ++-- test/orm/test_cache_key.py | 102 ++++++++++++++++++++- 3 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/8401.rst diff --git a/doc/build/changelog/unreleased_14/8401.rst b/doc/build/changelog/unreleased_14/8401.rst new file mode 100644 index 0000000000..119c6cff1a --- /dev/null +++ b/doc/build/changelog/unreleased_14/8401.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: orm, bug + :tickets: 8401 + + Hardened the cache key strategy for the :func:`_orm.aliased` and + :func:`_orm.with_polymorphic` constructs. While no issue involving actual + statements being cached can easily be demonstrated (if at all), these two + constructs were not including enough of what makes them unique in their + cache keys for caching on the aliased construct alone to be accurate. diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 02080a27f9..9a5399af00 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -748,6 +748,19 @@ class AliasedInsp( "_nest_adapters", ) + _cache_key_traversal = [ + ("name", visitors.ExtendedInternalTraversal.dp_string), + ("_adapt_on_names", visitors.ExtendedInternalTraversal.dp_boolean), + ("_use_mapper_path", visitors.ExtendedInternalTraversal.dp_boolean), + ("_target", visitors.ExtendedInternalTraversal.dp_inspectable), + ("selectable", visitors.ExtendedInternalTraversal.dp_clauseelement), + ( + "with_polymorphic_mappers", + visitors.InternalTraversal.dp_has_cache_key_list, + ), + ("polymorphic_on", visitors.InternalTraversal.dp_clauseelement), + ] + mapper: Mapper[_O] selectable: FromClause _adapter: sql_util.ColumnAdapter @@ -940,12 +953,6 @@ class AliasedInsp( def entity_namespace(self) -> AliasedClass[_O]: return self.entity - _cache_key_traversal = [ - ("name", visitors.ExtendedInternalTraversal.dp_string), - ("_adapt_on_names", visitors.ExtendedInternalTraversal.dp_boolean), - ("selectable", visitors.ExtendedInternalTraversal.dp_clauseelement), - ] - @property def class_(self) -> Type[_O]: """Return the mapped class ultimately represented by this diff --git a/test/orm/test_cache_key.py b/test/orm/test_cache_key.py index 7beadc08cf..08fd22dc89 100644 --- a/test/orm/test_cache_key.py +++ b/test/orm/test_cache_key.py @@ -5,6 +5,7 @@ from sqlalchemy import Column from sqlalchemy import func from sqlalchemy import inspect from sqlalchemy import Integer +from sqlalchemy import literal_column from sqlalchemy import null from sqlalchemy import select from sqlalchemy import Table @@ -63,8 +64,19 @@ class CacheKeyTest(CacheKeyFixture, _fixtures.FixtureTest): def test_mapper_and_aliased(self): User, Address, Keyword = self.classes("User", "Address", "Keyword") + addresses_table = self.tables.addresses + self._run_cache_key_fixture( - lambda: (inspect(User), inspect(Address), inspect(aliased(User))), + lambda: ( + inspect(User), + inspect(Address), + inspect(aliased(User)), + inspect(aliased(aliased(User, addresses_table))), + inspect(aliased(aliased(User), addresses_table.select())), + inspect(aliased(Address)), + inspect(aliased(Address, addresses_table.select())), + inspect(aliased(User, addresses_table.select())), + ), compare_values=True, ) @@ -611,6 +623,94 @@ class PolyCacheKeyTest(CacheKeyFixture, _poly_fixtures._Polymorphic): compare_values=True, ) + def test_wpoly_cache_keys(self): + Person, Manager, Engineer, Boss = self.classes( + "Person", "Manager", "Engineer", "Boss" + ) + + meb_stmt = inspect( + with_polymorphic(Person, [Manager, Engineer, Boss]) + ).selectable + me_stmt = inspect( + with_polymorphic(Person, [Manager, Engineer]) + ).selectable + + self._run_cache_key_fixture( + lambda: ( + inspect(Person), + inspect( + aliased(Person, me_stmt), + ), + inspect( + aliased(Person, meb_stmt), + ), + inspect( + with_polymorphic(Person, [Manager, Engineer]), + ), + # aliased=True is the same as flat=True for default selectable + inspect( + with_polymorphic( + Person, [Manager, Engineer], aliased=True + ), + ), + inspect( + with_polymorphic(Person, [Manager, Engineer], flat=True), + ), + inspect( + with_polymorphic( + Person, [Manager, Engineer], flat=True, innerjoin=True + ), + ), + inspect( + with_polymorphic( + Person, + [Manager, Engineer], + flat=True, + _use_mapper_path=True, + ), + ), + inspect( + with_polymorphic( + Person, + [Manager, Engineer], + flat=True, + adapt_on_names=True, + ), + ), + inspect( + with_polymorphic( + Person, [Manager, Engineer], selectable=meb_stmt + ), + ), + inspect( + with_polymorphic( + Person, + [Manager, Engineer], + selectable=meb_stmt, + aliased=True, + ), + ), + inspect( + with_polymorphic(Person, [Manager, Engineer, Boss]), + ), + inspect( + with_polymorphic( + Person, + [Manager, Engineer, Boss], + polymorphic_on=literal_column("foo"), + ), + ), + inspect( + with_polymorphic( + Person, + [Manager, Engineer, Boss], + polymorphic_on=literal_column("bar"), + ), + ), + ), + compare_values=True, + ) + def test_wp_queries(self): Person, Manager, Engineer, Boss = self.classes( "Person", "Manager", "Engineer", "Boss" -- 2.47.2