From 770e1e399cb0c91db73a551e1962ccbb57f42e76 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 15 Jun 2017 12:03:22 -0400 Subject: [PATCH] Repair regression to pathing for subclasses Issue #3963's initial commit narrowed the "current path" match rules too much such that a path that matches current path on subclass would no longer match. Change-Id: I8c9a0db91a09d789cfb8666288a913f8bbcdb2e9 Fixes: #3963 --- lib/sqlalchemy/orm/strategy_options.py | 9 +++- test/orm/test_options.py | 66 +++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index d3f456969b..4c7e34ebbf 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -472,7 +472,7 @@ class _UnboundLoad(Load): def _chop_path(self, to_chop, path): i = -1 - for i, (c_token, (p_mapper, p_prop)) in enumerate( + for i, (c_token, (p_entity, p_prop)) in enumerate( zip(to_chop, path.pairs())): if isinstance(c_token, util.string_types): if i == 0 and c_token.endswith(':' + _DEFAULT_TOKEN): @@ -482,7 +482,12 @@ class _UnboundLoad(Load): return None elif isinstance(c_token, PropComparator): if c_token.property is not p_prop or \ - c_token._parententity is not p_mapper: + ( + c_token._parententity is not p_entity and ( + not c_token._parententity.is_mapper or + not c_token._parententity.isa(p_entity) + ) + ): return None else: i += 1 diff --git a/test/orm/test_options.py b/test/orm/test_options.py index e29a63e08d..053ba5e4b7 100644 --- a/test/orm/test_options.py +++ b/test/orm/test_options.py @@ -9,7 +9,9 @@ import sqlalchemy as sa from sqlalchemy import testing from sqlalchemy.testing.assertions import eq_, assert_raises_message from test.orm import _fixtures - +from sqlalchemy import Column, Integer, String, ForeignKey +from sqlalchemy.orm import subqueryload +from sqlalchemy.testing import fixtures class QueryTest(_fixtures.FixtureTest): run_setup_mappers = 'once' @@ -572,6 +574,68 @@ class OptionsTest(PathTest, QueryTest): ]) +class FromSubclassOptionsTest(PathTest, fixtures.DeclarativeMappedTest): + # test for regression to #3963 + run_setup_mappers = 'once' + run_inserts = 'once' + run_deletes = None + + @classmethod + def setup_mappers(cls): + Base = cls.DeclarativeBasic + + class BaseCls(Base): + __tablename__ = 'basecls' + id = Column(Integer, primary_key=True) + + type = Column(String) + related_id = Column(ForeignKey('related.id')) + related = relationship("Related") + + class SubClass(BaseCls): + __tablename__ = 'subcls' + id = Column(ForeignKey('basecls.id'), primary_key=True) + + class Related(Base): + __tablename__ = 'related' + id = Column(Integer, primary_key=True) + + sub_related_id = Column(ForeignKey('sub_related.id')) + sub_related = relationship('SubRelated') + + class SubRelated(Base): + __tablename__ = 'sub_related' + id = Column(Integer, primary_key=True) + + def _assert_path_result(self, opt, q, paths): + q._attributes = q._attributes.copy() + attr = {} + + for val in opt._to_bind: + val._bind_loader( + [ent.entity_zero for ent in q._mapper_entities], + q._current_path, attr, False) + + assert_paths = [k[1] for k in attr] + eq_( + set([p for p in assert_paths]), + set([self._make_path(p) for p in paths]) + ) + + def test_with_current_nonmatching_entity_subclasses(self): + BaseCls, SubClass, Related, SubRelated = self.classes( + 'BaseCls', 'SubClass', 'Related', 'SubRelated') + sess = Session() + + q = sess.query(Related)._with_current_path( + self._make_path_registry( + [inspect(SubClass), 'related']) + ) + + opt = subqueryload(SubClass.related).subqueryload(Related.sub_related) + self._assert_path_result(opt, q, [(Related, "sub_related")]) + + class OptionsNoPropTest(_fixtures.FixtureTest): """test the error messages emitted when using property options in conjunction with column-only entities, or -- 2.39.5