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

diff --git a/doc/build/changelog/unreleased_14/7545.rst b/doc/build/changelog/unreleased_14/7545.rst
new file mode 100644 (file)
index 0000000..ea31d1a
--- /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 36b97cf17e0532db8f94b4f28cb9162c7a5e9784..5d784498a0e2ddf90b2814938a16e890dbb48c7d 100644 (file)
--- 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 ca656a8682ea6129a4fd24a6aeb9762d3e5febb4..d6f1532ee5c39fa8c8b1af20d6c0cd4fcc21a1f4 100644 (file)
--- 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 fb27c910ffc6cd8b55e79c6b201d656f487649ca..dcb0a5e749cb6ae1722fc3310d96d5c3b5fee156 100644 (file)
--- 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 5c5b481db14ee63627bd532cbffee5b0558138d2..6ad2f209724bee55d93d15b86485aad4196565b1 100644 (file)
--- a/test/orm/declarative/test_typed_mapping.py
+++ b/test/orm/declarative/test_typed_mapping.py
@@ -1822,6 +1822,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]
@@ -1835,6 +1836,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 87c61d3952db60e339711de1d329484354760346..9d7b73e1cf829fc63e377ecdf9776b2318dda10c 100644 (file)
--- 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 75955afb5a5e4c0b4e6b2370ea0b25bc36fd9700..47609daa064dc9f2c7592363b21568d0494cb10e 100644 (file)
--- 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())