From: Mike Bayer Date: Mon, 10 Oct 2022 18:03:04 +0000 (-0400) Subject: warn for no polymorphic identity w/ poly hierarchy X-Git-Tag: rel_1_4_42~6^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=855472e963bcebe426f1fb89e1b82947e4f5208f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git warn for no polymorphic identity w/ poly hierarchy A warning is emitted when attempting to configure a mapped class within an inheritance hierarchy where the mapper is not given any polymorphic identity, however there is a polymorphic discriminator column assigned. Such classes should be abstract if they never intend to load directly. Fixes: #7545 Change-Id: I94f04e59736c73e3f39d883a75d763e3f06ecc3d (cherry picked from commit bf0634131115a76aaca52eebd3c7d3fb52f8258b) --- diff --git a/doc/build/changelog/unreleased_14/7545.rst b/doc/build/changelog/unreleased_14/7545.rst new file mode 100644 index 0000000000..ea31d1a3c3 --- /dev/null +++ b/doc/build/changelog/unreleased_14/7545.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 7545 + + A warning is emitted when attempting to configure a mapped class within an + inheritance hierarchy where the mapper is not given any polymorphic + identity, however there is a polymorphic discriminator column assigned. + Such classes should be abstract if they never intend to load directly. + diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 2554dde0de..15eae2dac0 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1079,10 +1079,27 @@ class Mapper( else: self.persist_selectable = self.local_table - if self.polymorphic_identity is not None and not self.concrete: - self._identity_class = self.inherits._identity_class - else: + if self.polymorphic_identity is None: + self._identity_class = self.class_ + + if self.inherits.base_mapper.polymorphic_on is not None: + util.warn( + "Mapper %s does not indicate a polymorphic_identity, " + "yet is part of an inheritance hierarchy that has a " + "polymorphic_on column of '%s'. Objects of this type " + "cannot be loaded polymorphically which can lead to " + "degraded or incorrect loading behavior in some " + "scenarios. Please establish a polmorphic_identity " + "for this class, or leave it un-mapped. " + "To omit mapping an intermediary class when using " + "declarative, set the '__abstract__ = True' " + "attribute on that class." + % (self, self.inherits.base_mapper.polymorphic_on) + ) + elif self.concrete: self._identity_class = self.class_ + else: + self._identity_class = self.inherits._identity_class if self.version_id_col is None: self.version_id_col = self.inherits.version_id_col diff --git a/test/orm/declarative/test_inheritance.py b/test/orm/declarative/test_inheritance.py index 7e43e25595..7f1b47f375 100644 --- a/test/orm/declarative/test_inheritance.py +++ b/test/orm/declarative/test_inheritance.py @@ -38,6 +38,7 @@ class DeclarativeTestBase(fixtures.TestBase, testing.AssertsExecutionResults): class DeclarativeInheritanceTest(DeclarativeTestBase): + @testing.emits_warning(r".*does not indicate a polymorphic_identity") def test_we_must_copy_mapper_args(self): class Person(Base): @@ -673,6 +674,9 @@ class DeclarativeInheritanceTest(DeclarativeTestBase): __tablename__ = "employee" id = Column(Integer, ForeignKey(Person.id), primary_key=True) + __mapper_args__ = { + "polymorphic_identity": "employee", + } class Engineer(Employee): __mapper_args__ = {"polymorphic_identity": "engineer"} @@ -1007,9 +1011,15 @@ class DeclarativeInheritanceTest(DeclarativeTestBase): __mapper_args__ = {"polymorphic_identity": "manager"} id = Column(Integer, ForeignKey("people.id"), primary_key=True) golf_swing = Column(String(50)) + __mapper_args__ = { + "polymorphic_identity": "manager", + } class Boss(Manager): boss_name = Column(String(50)) + __mapper_args__ = { + "polymorphic_identity": "boss", + } is_( Boss.__mapper__.column_attrs["boss_name"].columns[0], diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index 6285a80a7f..9daafb7cef 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -32,6 +32,7 @@ from sqlalchemy.sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from sqlalchemy.testing import assert_raises from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import eq_ +from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ from sqlalchemy.testing import mock @@ -3414,7 +3415,12 @@ class NoPolyIdentInMiddleTest(fixtures.MappedTest): cls.mapper_registry.map_imperatively( A, base, polymorphic_on=base.c.type ) - cls.mapper_registry.map_imperatively(B, inherits=A) + + with expect_warnings( + r"Mapper mapped class B->base does not indicate a " + "polymorphic_identity," + ): + cls.mapper_registry.map_imperatively(B, inherits=A) cls.mapper_registry.map_imperatively( C, inherits=B, polymorphic_identity="c" ) @@ -3424,6 +3430,28 @@ class NoPolyIdentInMiddleTest(fixtures.MappedTest): cls.mapper_registry.map_imperatively( E, inherits=A, polymorphic_identity="e" ) + cls.mapper_registry.configure() + + def test_warning(self, decl_base): + """test #7545""" + + class A(decl_base): + __tablename__ = "a" + id = Column(Integer, primary_key=True) + type = Column(String) + + __mapper_args__ = {"polymorphic_on": type} + + class B(A): + __mapper_args__ = {"polymorphic_identity": "b"} + + with expect_warnings( + r"Mapper mapped class C->a does not indicate a " + "polymorphic_identity," + ): + + class C(A): + __mapper_args__ = {} def test_load_from_middle(self): C, B = self.classes.C, self.classes.B diff --git a/test/orm/test_events.py b/test/orm/test_events.py index 5026551004..efb39bd2fd 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -1209,6 +1209,7 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): # not been loaded yet (Employer), and therefore cannot be configured: class Mammal(Animal): nonexistent = relationship("Nonexistent") + __mapper_args__ = {"polymorphic_identity": "mammal"} # These new classes should not be configured at this point: unconfigured = list(mapperlib._unconfigured_mappers())