diff options
-rw-r--r-- | doc/build/changelog/unreleased_20/9777.rst | 11 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/strategies.py | 73 | ||||
-rw-r--r-- | test/orm/test_options.py | 19 |
3 files changed, 48 insertions, 55 deletions
diff --git a/doc/build/changelog/unreleased_20/9777.rst b/doc/build/changelog/unreleased_20/9777.rst new file mode 100644 index 000000000..0f043f9be --- /dev/null +++ b/doc/build/changelog/unreleased_20/9777.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, orm + :tickets: 9777 + + Modified the ``JoinedLoader`` implementation to use a simpler approach in + one particular area where it previously used a cached structure that would + be shared among threads. The rationale is to avoid a potential race + condition which is suspected of being the cause of a particular crash + that's been reported multiple times. The cached structure in question is + still ultimately "cached" via the compiled SQL cache, so a performance + degradation is not anticipated. diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 8e06c4f59..96b1d053e 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -2097,12 +2097,11 @@ class JoinedLoader(AbstractRelationshipLoader): """ - __slots__ = "join_depth", "_aliased_class_pool" + __slots__ = "join_depth" def __init__(self, parent, strategy_key): super().__init__(parent, strategy_key) self.join_depth = self.parent_property.join_depth - self._aliased_class_pool = [] def init_class_attribute(self, mapper): self.parent_property._get_strategy( @@ -2215,14 +2214,19 @@ class JoinedLoader(AbstractRelationshipLoader): chained_from_outerjoin=chained_from_outerjoin, ) - if with_poly_entity is not None and None in set( - compile_state.secondary_columns - ): - raise sa_exc.InvalidRequestError( - "Detected unaliased columns when generating joined " - "load. Make sure to use aliased=True or flat=True " - "when using joined loading with with_polymorphic()." - ) + has_nones = util.NONE_SET.intersection(compile_state.secondary_columns) + + if has_nones: + if with_poly_entity is not None: + raise sa_exc.InvalidRequestError( + "Detected unaliased columns when generating joined " + "load. Make sure to use aliased=True or flat=True " + "when using joined loading with with_polymorphic()." + ) + else: + compile_state.secondary_columns = [ + c for c in compile_state.secondary_columns if c is not None + ] def _init_user_defined_eager_proc( self, loadopt, compile_state, target_attributes @@ -2308,40 +2312,6 @@ class JoinedLoader(AbstractRelationshipLoader): add_to_collection = context.primary_columns return user_defined_adapter, adapter, add_to_collection - def _gen_pooled_aliased_class(self, context): - # keep a local pool of AliasedClass objects that get re-used. - # we need one unique AliasedClass per query per appearance of our - # entity in the query. - - if inspect(self.entity).is_aliased_class: - alt_selectable = inspect(self.entity).selectable - else: - alt_selectable = None - - key = ("joinedloader_ac", self) - if key not in context.attributes: - context.attributes[key] = idx = 0 - else: - context.attributes[key] = idx = context.attributes[key] + 1 - - if idx >= len(self._aliased_class_pool): - to_adapt = orm_util.AliasedClass( - self.mapper, - alias=alt_selectable._anonymous_fromclause(flat=True) - if alt_selectable is not None - else None, - flat=True, - use_mapper_path=True, - ) - - # load up the .columns collection on the Alias() before - # the object becomes shared among threads. this prevents - # races for column identities. - inspect(to_adapt).selectable.c - self._aliased_class_pool.append(to_adapt) - - return self._aliased_class_pool[idx] - def _generate_row_adapter( self, compile_state, @@ -2359,7 +2329,20 @@ class JoinedLoader(AbstractRelationshipLoader): if with_poly_entity: to_adapt = with_poly_entity else: - to_adapt = self._gen_pooled_aliased_class(compile_state) + insp = inspect(self.entity) + if insp.is_aliased_class: + alt_selectable = insp.selectable + else: + alt_selectable = None + + to_adapt = orm_util.AliasedClass( + self.mapper, + alias=alt_selectable._anonymous_fromclause(flat=True) + if alt_selectable is not None + else None, + flat=True, + use_mapper_path=True, + ) to_adapt_insp = inspect(to_adapt) diff --git a/test/orm/test_options.py b/test/orm/test_options.py index 47ffedb07..3b088b998 100644 --- a/test/orm/test_options.py +++ b/test/orm/test_options.py @@ -28,6 +28,7 @@ from sqlalchemy.orm import undefer from sqlalchemy.orm import util as orm_util from sqlalchemy.orm import with_polymorphic from sqlalchemy.testing import fixtures +from sqlalchemy.testing import is_not from sqlalchemy.testing.assertions import assert_raises_message from sqlalchemy.testing.assertions import AssertsCompiledSQL from sqlalchemy.testing.assertions import emits_warning @@ -1288,20 +1289,18 @@ class PickleTest(fixtures.MappedTest): def test_pickle_relationship_loader(self, user_address_fixture): User, Address = user_address_fixture - for i in range(3): - opt = joinedload(User.addresses) - - q1 = fixture_session().query(User).options(opt) - c1 = q1._compile_context() + opt = joinedload(User.addresses) - pickled = pickle.dumps(opt) + pickled = pickle.dumps(opt) - opt2 = pickle.loads(pickled) + opt2 = pickle.loads(pickled) - q2 = fixture_session().query(User).options(opt2) - c2 = q2._compile_context() + is_not(opt, opt2) + assert isinstance(opt, Load) + assert isinstance(opt2, Load) - eq_(c1.attributes, c2.attributes) + for k in opt.__slots__: + eq_(getattr(opt, k), getattr(opt2, k)) class LocalOptsTest(PathTest, QueryTest): |