From 3b77079321d35f63103ead5092d9eeeb5ef546ca Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 10 Jan 2022 22:11:50 -0500 Subject: [PATCH] change state.load_options to a tuple having this be an immutable sequence is safer and possibly lower overhead. The change here went in with no issues save for tests that asserted it was a set. InstanceState.load_options is only referred towards when the object is first loaded, and then within the logic that emits an object refresh as well as within a lazy loader. it's only accessed as a whole collection. Fixes: #7558 Change-Id: Id1adbec0f93bcfbfc934ec9cd39e71e74727845d --- lib/sqlalchemy/orm/context.py | 6 +++--- lib/sqlalchemy/orm/state.py | 2 +- lib/sqlalchemy/orm/strategies.py | 2 +- test/orm/inheritance/test_poly_loading.py | 2 +- test/orm/test_merge.py | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py index 8ec18b8651..cba7cf07dc 100644 --- a/lib/sqlalchemy/orm/context.py +++ b/lib/sqlalchemy/orm/context.py @@ -103,7 +103,7 @@ class QueryContext: self.loaders_require_uniquing = False self.params = params - self.propagated_loader_options = { + self.propagated_loader_options = tuple( # issue 7447. # propagated loader options will be present on loaded InstanceState # objects under state.load_options and are typically used by @@ -118,14 +118,14 @@ class QueryContext: cached_o for cached_o in compile_state.select_statement._with_options if cached_o.propagate_to_loaders and cached_o._is_compile_state - } | { + ) + tuple( # for user defined loader options that are not "compile state", # those just need to be present as they are uncached_o for uncached_o in statement._with_options if uncached_o.propagate_to_loaders and not uncached_o._is_compile_state - } + ) self.attributes = dict(compile_state.attributes) diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index e0eadefc4e..01ee16a13f 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/lib/sqlalchemy/orm/state.py @@ -68,7 +68,7 @@ class InstanceState(interfaces.InspectionAttrInfo): session_id = None key = None runid = None - load_options = util.EMPTY_SET + load_options = () load_path = PathRegistry.root insert_order = None _strong_obj = None diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index a8a5639528..beaf649b78 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -964,7 +964,7 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): if state.load_options or (loadopt and loadopt._extra_criteria): effective_path = state.load_path[self.parent_property] - opts = tuple(state.load_options) + opts = state.load_options if loadopt and loadopt._extra_criteria: use_get = False diff --git a/test/orm/inheritance/test_poly_loading.py b/test/orm/inheritance/test_poly_loading.py index 517431e705..5f8ff56395 100644 --- a/test/orm/inheritance/test_poly_loading.py +++ b/test/orm/inheritance/test_poly_loading.py @@ -928,4 +928,4 @@ class LazyLoaderTransfersOptsTest(fixtures.DeclarativeMappedTest): u = sess.execute(select(User).options(*opts)).scalars().one() address = u.address - eq_(inspect(address).load_options, set(opts)) + eq_(inspect(address).load_options, opts) diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py index 0f29cfc568..dc04d4da65 100644 --- a/test/orm/test_merge.py +++ b/test/orm/test_merge.py @@ -1583,7 +1583,7 @@ class MergeTest(_fixtures.FixtureTest): for u in s1_users: ustate = attributes.instance_state(u) eq_(ustate.load_path.path, (umapper,)) - eq_(ustate.load_options, set()) + eq_(ustate.load_options, ()) for u in s2_users: sess.merge(u) @@ -1591,7 +1591,7 @@ class MergeTest(_fixtures.FixtureTest): for u in s1_users: ustate = attributes.instance_state(u) eq_(ustate.load_path.path, (umapper,)) - eq_(ustate.load_options, set([opt2])) + eq_(ustate.load_options, (opt2,)) # test 2. present options are replaced by merge options sess = fixture_session() @@ -1599,7 +1599,7 @@ class MergeTest(_fixtures.FixtureTest): for u in s1_users: ustate = attributes.instance_state(u) eq_(ustate.load_path.path, (umapper,)) - eq_(ustate.load_options, set([opt1])) + eq_(ustate.load_options, (opt1,)) for u in s2_users: sess.merge(u) @@ -1607,7 +1607,7 @@ class MergeTest(_fixtures.FixtureTest): for u in s1_users: ustate = attributes.instance_state(u) eq_(ustate.load_path.path, (umapper,)) - eq_(ustate.load_options, set([opt2])) + eq_(ustate.load_options, (opt2,)) def test_resolve_conflicts_pending_doesnt_interfere_no_ident(self): User, Address, Order = ( -- 2.47.2