From: Mike Bayer Date: Mon, 20 Jan 2020 17:41:22 +0000 (-0500) Subject: Adjust natural path to relationship's base mapper for aliased class also X-Git-Tag: rel_1_3_13~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2702eeffb98192de560c1845b5864dcda8c934fe;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Adjust natural path to relationship's base mapper for aliased class also Fixed regression in loader options introduced in 1.3.0b3 via :ticket:`4468` where the ability to create a loader option using :meth:`.PropComparator.of_type` targeting an aliased entity that is an inheriting subclass of the entity which the preceding relationship refers to would fail to produce a matching path. See also :ticket:`5082` fixed in this same release which involves a similar kind of issue. Fixes: #5107 Change-Id: I5c6717b925060c3f8da42190d1f00d05248befd8 (cherry picked from commit c9bf876d93b9389d6d3b619565f6da166bab5ec2) --- diff --git a/doc/build/changelog/unreleased_13/5107.rst b/doc/build/changelog/unreleased_13/5107.rst new file mode 100644 index 0000000000..1778f48a75 --- /dev/null +++ b/doc/build/changelog/unreleased_13/5107.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: orm, bug + :tickets: 5107 + + Fixed regression in loader options introduced in 1.3.0b3 via :ticket:`4468` + where the ability to create a loader option using + :meth:`.PropComparator.of_type` targeting an aliased entity that is an + inheriting subclass of the entity which the preceding relationship refers + to would fail to produce a matching path. See also :ticket:`5082` fixed + in this same release which involves a similar kind of issue. diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py index cd6815717a..491bf3337b 100644 --- a/lib/sqlalchemy/orm/path_registry.py +++ b/lib/sqlalchemy/orm/path_registry.py @@ -242,51 +242,61 @@ class PropRegistry(PathRegistry): # given MapperProperty's parent. insp = inspection.inspect(parent[-1]) natural_parent = parent + if not insp.is_aliased_class or insp._use_mapper_path: parent = natural_parent = parent.parent[prop.parent] - elif insp.is_aliased_class and insp.with_polymorphic_mappers: - if ( - prop.parent is not insp.mapper - and prop.parent in insp.with_polymorphic_mappers - ): - subclass_entity = parent[-1]._entity_for_mapper(prop.parent) - parent = parent.parent[subclass_entity] - - # when building a path where with_polymorphic() is in use, - # special logic to determine the "natural path" when subclass - # entities are used. - # - # here we are trying to distinguish between a path that starts - # on a the with_polymorhpic entity vs. one that starts on a - # normal entity that introduces a with_polymorphic() in the - # middle using of_type(): - # - # # as in test_polymorphic_rel-> - # # test_subqueryload_on_subclass_uses_path_correctly - # wp = with_polymorphic(RegularEntity, "*") - # sess.query(wp).options(someload(wp.SomeSubEntity.foos)) - # - # vs - # - # # as in test_relationship->JoinedloadWPolyOfTypeContinued - # wp = with_polymorphic(SomeFoo, "*") - # sess.query(RegularEntity).options( - # someload(RegularEntity.foos.of_type(wp)) - # .someload(wp.SubFoo.bar) - # ) - # - # in the former case, the Query as it generates a path that we - # want to match will be in terms of the with_polymorphic at the - # beginning. in the latter case, Query will generate simple - # paths that don't know about this with_polymorphic, so we must - # use a separate natural path. - # - # - if parent.parent: - natural_parent = parent.parent[subclass_entity.mapper] - self.is_unnatural = True - else: - natural_parent = parent + elif ( + insp.is_aliased_class + and insp.with_polymorphic_mappers + and prop.parent is not insp.mapper + and prop.parent in insp.with_polymorphic_mappers + ): + subclass_entity = parent[-1]._entity_for_mapper(prop.parent) + parent = parent.parent[subclass_entity] + + # when building a path where with_polymorphic() is in use, + # special logic to determine the "natural path" when subclass + # entities are used. + # + # here we are trying to distinguish between a path that starts + # on a the with_polymorhpic entity vs. one that starts on a + # normal entity that introduces a with_polymorphic() in the + # middle using of_type(): + # + # # as in test_polymorphic_rel-> + # # test_subqueryload_on_subclass_uses_path_correctly + # wp = with_polymorphic(RegularEntity, "*") + # sess.query(wp).options(someload(wp.SomeSubEntity.foos)) + # + # vs + # + # # as in test_relationship->JoinedloadWPolyOfTypeContinued + # wp = with_polymorphic(SomeFoo, "*") + # sess.query(RegularEntity).options( + # someload(RegularEntity.foos.of_type(wp)) + # .someload(wp.SubFoo.bar) + # ) + # + # in the former case, the Query as it generates a path that we + # want to match will be in terms of the with_polymorphic at the + # beginning. in the latter case, Query will generate simple + # paths that don't know about this with_polymorphic, so we must + # use a separate natural path. + # + # + if parent.parent: + natural_parent = parent.parent[subclass_entity.mapper] + self.is_unnatural = True + else: + natural_parent = parent + elif ( + natural_parent.parent + and insp.is_aliased_class + and prop.parent # this should always be the case here + is not insp.mapper + and insp.mapper.isa(prop.parent) + ): + natural_parent = parent.parent[prop.parent] self.prop = prop self.parent = parent diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py index da7136fc98..b005b6b40f 100644 --- a/test/orm/inheritance/test_relationship.py +++ b/test/orm/inheritance/test_relationship.py @@ -1825,6 +1825,70 @@ class JoinedloadWPolyOfTypeContinued( self.assert_sql_count(testing.db, go, 1) +class ContainsEagerMultipleOfType( + fixtures.DeclarativeMappedTest, testing.AssertsCompiledSQL +): + """test for #5107 """ + + __dialect__ = "default" + + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class X(Base): + __tablename__ = "x" + id = Column(Integer, primary_key=True) + a_id = Column(Integer, ForeignKey("a.id")) + a = relationship("A", back_populates="x") + + class A(Base): + __tablename__ = "a" + id = Column(Integer, primary_key=True) + b = relationship("B", back_populates="a") + kind = Column(String(30)) + x = relationship("X", back_populates="a") + __mapper_args__ = { + "polymorphic_identity": "a", + "polymorphic_on": kind, + "with_polymorphic": "*", + } + + class B(A): + a_id = Column(Integer, ForeignKey("a.id")) + a = relationship( + "A", back_populates="b", uselist=False, remote_side=A.id + ) + __mapper_args__ = {"polymorphic_identity": "b"} + + def test_contains_eager_multi_alias(self): + X, B, A = self.classes("X", "B", "A") + s = Session() + + a_b_alias = aliased(B, name="a_b") + b_x_alias = aliased(X, name="b_x") + + q = ( + s.query(A) + .outerjoin(A.b.of_type(a_b_alias)) + .outerjoin(a_b_alias.x.of_type(b_x_alias)) + .options( + contains_eager(A.b.of_type(a_b_alias)).contains_eager( + a_b_alias.x.of_type(b_x_alias) + ) + ) + ) + self.assert_compile( + q, + "SELECT b_x.id AS b_x_id, b_x.a_id AS b_x_a_id, a_b.id AS a_b_id, " + "a_b.kind AS a_b_kind, a_b.a_id AS a_b_a_id, a.id AS a_id_1, " + "a.kind AS a_kind, a.a_id AS a_a_id FROM a " + "LEFT OUTER JOIN a AS a_b ON a.id = a_b.a_id AND a_b.kind IN " + "(:kind_1) LEFT OUTER JOIN x AS b_x " + "ON a_b.id = b_x.a_id", + ) + + class JoinedloadSinglePolysubSingle( fixtures.DeclarativeMappedTest, testing.AssertsCompiledSQL ): diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py index c274c05410..680862b7b9 100644 --- a/test/orm/test_utils.py +++ b/test/orm/test_utils.py @@ -968,6 +968,89 @@ class PathRegistryInhTest(_poly_fixtures._Polymorphic): ), ) + def test_nonpoly_oftype_aliased_subclass_onroot(self): + Engineer = _poly_fixtures.Engineer + eng_alias = aliased(Engineer) + ea_insp = inspect(eng_alias) + + p1 = PathRegistry.coerce((ea_insp, ea_insp.mapper.attrs.paperwork)) + + eq_(p1.path, (ea_insp, ea_insp.mapper.attrs.paperwork)) + eq_(p1.natural_path, (ea_insp, ea_insp.mapper.attrs.paperwork)) + + def test_nonpoly_oftype_aliased_subclass(self): + Company = _poly_fixtures.Company + Person = _poly_fixtures.Person + Engineer = _poly_fixtures.Engineer + cmapper = inspect(Company) + pmapper = inspect(Person) + eng_alias = aliased(Engineer) + ea_insp = inspect(eng_alias) + + p1 = PathRegistry.coerce( + ( + cmapper, + cmapper.attrs.employees, + ea_insp, + ea_insp.mapper.attrs.paperwork, + ) + ) + + eq_( + p1.path, + ( + cmapper, + cmapper.attrs.employees, + ea_insp, + ea_insp.mapper.attrs.paperwork, + ), + ) + eq_( + p1.natural_path, + ( + cmapper, + cmapper.attrs.employees, + pmapper, + pmapper.attrs.paperwork, + ), + ) + + def test_nonpoly_oftype_subclass(self): + Company = _poly_fixtures.Company + Person = _poly_fixtures.Person + Engineer = _poly_fixtures.Engineer + emapper = inspect(Engineer) + cmapper = inspect(Company) + pmapper = inspect(Person) + + p1 = PathRegistry.coerce( + ( + cmapper, + cmapper.attrs.employees, + emapper, + emapper.attrs.paperwork, + ) + ) + + eq_( + p1.path, + ( + cmapper, + cmapper.attrs.employees, + pmapper, + pmapper.attrs.paperwork, + ), + ) + eq_( + p1.natural_path, + ( + cmapper, + cmapper.attrs.employees, + pmapper, + pmapper.attrs.paperwork, + ), + ) + def test_with_poly_base(self): Person = _poly_fixtures.Person Engineer = _poly_fixtures.Engineer