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 (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 2554dde0de8c7f6cab8e281b86c534c209689256..15eae2dac094fdf0d5133b607d47f208d67505e2 100644 (file)
--- 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 7e43e255954d203a29ce097e424b9a2cc0304dcf..7f1b47f37582ac7b91a7b1d0d9f76dd0c5f347e8 100644 (file)
--- 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 6285a80a7f3a8b690acec8f318c92b83026ce2e6..9daafb7cefb371649614fe63620d4b0657bf8e56 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
@@ -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 50265510042505984e19272de2eb58a679d8bf88..efb39bd2fdc6ae0a1ac80472bf882449d34c1ff5 100644 (file)
--- 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())