From 9cd7cca2c70e87c852af7e570aabdfa7463ce645 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 (cherry picked from commit a58f1b9c698dc7be29d43f2c4c21de8918943f77) --- doc/build/changelog/unreleased_14/8401.rst | 9 ++ lib/sqlalchemy/orm/util.py | 19 ++-- test/orm/test_cache_key.py | 102 ++++++++++++++++++++- test/sql/test_resultset.py | 1 + 4 files changed, 124 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 56aa9ff6c7..6f3278ed78 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -650,6 +650,19 @@ class AliasedInsp( """ + _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), + ] + def __init__( self, entity, @@ -756,12 +769,6 @@ class AliasedInsp( def entity_namespace(self): 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): """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 f42e59216a..daf963952c 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, ) @@ -606,6 +618,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" diff --git a/test/sql/test_resultset.py b/test/sql/test_resultset.py index 13ffc5eebd..13190f915f 100644 --- a/test/sql/test_resultset.py +++ b/test/sql/test_resultset.py @@ -732,6 +732,7 @@ class CursorResultTest(fixtures.TablesTest): lambda: r._mapping["foo"], ) + @testing.skip_if("+aiosqlite", "unknown issue") @testing.combinations( (True,), (False,), -- 2.47.2