diff options
| -rw-r--r-- | doc/build/changelog/unreleased_14/7545.rst | 9 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 23 | ||||
| -rw-r--r-- | test/orm/declarative/test_dc_transforms.py | 2 | ||||
| -rw-r--r-- | test/orm/declarative/test_inheritance.py | 10 | ||||
| -rw-r--r-- | test/orm/declarative/test_typed_mapping.py | 2 | ||||
| -rw-r--r-- | test/orm/inheritance/test_basic.py | 30 | ||||
| -rw-r--r-- | test/orm/test_events.py | 1 |
7 files changed, 73 insertions, 4 deletions
diff --git a/doc/build/changelog/unreleased_14/7545.rst b/doc/build/changelog/unreleased_14/7545.rst new file mode 100644 index 000000000..ea31d1a3c --- /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 36b97cf17..5d784498a 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1224,10 +1224,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_dc_transforms.py b/test/orm/declarative/test_dc_transforms.py index ca656a868..d6f1532ee 100644 --- a/test/orm/declarative/test_dc_transforms.py +++ b/test/orm/declarative/test_dc_transforms.py @@ -303,6 +303,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): status: Mapped[str] = mapped_column(String(30)) engineer_name: Mapped[str] primary_language: Mapped[str] + __mapper_args__ = {"polymorphic_identity": "engineer"} e1 = Engineer("nm", "st", "en", "pl") eq_(e1.name, "nm") @@ -451,6 +452,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): status: Mapped[str] = mapped_column(String(30)) engineer_name: Mapped[str] primary_language: Mapped[str] + __mapper_args__ = {"polymorphic_identity": "engineer"} e1 = Engineer("st", "en", "pl") eq_(e1.status, "st") diff --git a/test/orm/declarative/test_inheritance.py b/test/orm/declarative/test_inheritance.py index fb27c910f..dcb0a5e74 100644 --- a/test/orm/declarative/test_inheritance.py +++ b/test/orm/declarative/test_inheritance.py @@ -40,6 +40,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): @@ -694,6 +695,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"} @@ -1028,9 +1032,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/declarative/test_typed_mapping.py b/test/orm/declarative/test_typed_mapping.py index 7ef35a850..f06a608a8 100644 --- a/test/orm/declarative/test_typed_mapping.py +++ b/test/orm/declarative/test_typed_mapping.py @@ -1820,6 +1820,7 @@ class AllYourFavoriteHitsTest(fixtures.TestBase, testing.AssertsCompiledSQL): person_id: Mapped[int] = mapped_column( ForeignKey("person.person_id"), primary_key=True ) + __mapper_args__ = {"polymorphic_identity": "engineer"} status: Mapped[str] = mapped_column(String(30)) engineer_name: Mapped[opt_str50] @@ -1833,6 +1834,7 @@ class AllYourFavoriteHitsTest(fixtures.TestBase, testing.AssertsCompiledSQL): ) status: Mapped[str] = mapped_column(String(30)) manager_name: Mapped[str50] + __mapper_args__ = {"polymorphic_identity": "manager"} is_(Person.__mapper__.polymorphic_on, Person.__table__.c.type) diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index 87c61d395..9d7b73e1c 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 @@ -3531,7 +3532,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 Mapper\[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" ) @@ -3541,6 +3547,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 Mapper\[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 75955afb5..47609daa0 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -1335,6 +1335,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()) |