Test instance for matching class hierarchy on get_from_identity

Fixed issue where a lazyload that uses session-local "get" against a target
many-to-one relationship where an object with the correct primary key is
present, however it's an instance of a sibling class, does not correctly
return None as is the case when the lazy loader actually emits a load for
that row.

Fixes: #5210
Change-Id: I89f9946cfeba61d89a272435f76a5a082b1da30c
(cherry picked from commit 900402b9aa901bc9b1ae3f6b525f076076c52529)

diff --git a/doc/build/changelog/unreleased_13/5210.rst b/doc/build/changelog/unreleased_13/5210.rst
new file mode 100644 (file)
index 0000000..0a50ba0
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5210.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 5210
+
+    Fixed issue where a lazyload that uses session-local "get" against a target
+    many-to-one relationship where an object with the correct primary key is
+    present, however it's an instance of a sibling class, does not correctly
+    return None as is the case when the lazy loader actually emits a load for
+    that row.

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index f7416efb882c0298b625978256614e7496cc5b5d..a8e590cf4fe7acdb18c9825a655657a3f6427d28 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -34,6 +34,7 @@ from .base import NO_CHANGE  # noqa
 from .base import NO_RAISE
 from .base import NO_VALUE
 from .base import NON_PERSISTENT_OK  # noqa
+from .base import PASSIVE_CLASS_MISMATCH  # noqa
 from .base import PASSIVE_NO_FETCH
 from .base import PASSIVE_NO_FETCH_RELATED  # noqa
 from .base import PASSIVE_NO_INITIALIZE

diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py
index fab43e4cff5b1fcc1e85525db37c4b31ab857f20..7d28e1082e05cd4094beb9341af8078f182a2e75 100644 (file)
--- a/lib/sqlalchemy/orm/base.py
+++ b/lib/sqlalchemy/orm/base.py
@@ -26,6 +26,13 @@ PASSIVE_NO_RESULT = util.symbol(
     """,
 )
 
+PASSIVE_CLASS_MISMATCH = util.symbol(
+    "PASSIVE_CLASS_MISMATCH",
+    """Symbol indicating that an object is locally present for a given
+    primary key identity but it is not of the requested class.  The
+    return value is therefore None and no SQL should be emitted.""",
+)
+
 ATTR_WAS_SET = util.symbol(
     "ATTR_WAS_SET",
     """Symbol returned by a loader callable to indicate the

diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 23c0c4e49b6fcda988edddac5c3951b4a5b10689..06224b7f890b4a5bc7e944dd8420d83cf9aa216b 100644 (file)
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -155,7 +155,7 @@ def merge_result(querylib, query, iterator, load=True):
         session.autoflush = autoflush
 
 
-def get_from_identity(session, key, passive):
+def get_from_identity(session, mapper, key, passive):
     """Look up the given key in the given session's identity map,
     check the object for expired state if found.
 
@@ -165,6 +165,9 @@ def get_from_identity(session, key, passive):
 
         state = attributes.instance_state(instance)
 
+        if mapper.inherits and not state.mapper.isa(mapper):
+            return attributes.PASSIVE_CLASS_MISMATCH
+
         # expired - ensure it still exists
         if state.expired:
             if not passive & attributes.SQL_OK:

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 478e70283cb34e99e3ea76d0e90b945b20379746..3e975890e49145a03f48bce4912d2ad02801f7ee 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -1055,7 +1055,7 @@ class Query(object):
         key = mapper.identity_key_from_primary_key(
             primary_key_identity, identity_token=identity_token
         )
-        return loading.get_from_identity(self.session, key, passive)
+        return loading.get_from_identity(self.session, mapper, key, passive)
 
     def _get_impl(self, primary_key_identity, db_load_fn, identity_token=None):
         # convert composite types to individual args
@@ -1115,6 +1115,8 @@ class Query(object):
                 if not issubclass(instance.__class__, mapper.class_):
                     return None
                 return instance
+            elif instance is attributes.PASSIVE_CLASS_MISMATCH:
+                return None
 
         return db_load_fn(self, primary_key_identity)
 

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 4bd74988a5c8d00666f654f6def2af55d8404a4f..22976321aa4c25646d69c5fcfe1ab2c1a22142db 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -736,7 +736,10 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
             )
 
             if instance is not None:
-                return instance
+                if instance is attributes.PASSIVE_CLASS_MISMATCH:
+                    return None
+                else:
+                    return instance
             elif (
                 not passive & attributes.SQL_OK
                 or not passive & attributes.RELATED_OBJECT_OK

diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py
index 51eddee7194d037765425b30fc8d38c84a021c30..851ad60b872958005f5c6e7b89f57f6cd7a84ae6 100644 (file)
--- a/test/orm/inheritance/test_relationship.py
+++ b/test/orm/inheritance/test_relationship.py
@@ -18,6 +18,7 @@ from sqlalchemy.testing import AssertsCompiledSQL
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
+from sqlalchemy.testing.entities import ComparableEntity
 from sqlalchemy.testing.schema import Column
 from sqlalchemy.testing.schema import Table
 
@@ -2678,3 +2679,75 @@ class BetweenSubclassJoinWExtraJoinedLoad(
             "seen AS seen_1 ON people.id = seen_1.id LEFT OUTER JOIN "
             "seen AS seen_2 ON people_1.id = seen_2.id",
         )
+
+
+class M2ODontLoadSiblingTest(fixtures.DeclarativeMappedTest):
+    """test for #5210"""
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class Parent(Base, ComparableEntity):
+            __tablename__ = "parents"
+
+            id = Column(Integer, primary_key=True)
+            child_type = Column(String(50), nullable=False)
+
+            __mapper_args__ = {
+                "polymorphic_on": child_type,
+            }
+
+        class Child1(Parent):
+            __tablename__ = "children_1"
+
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+
+            __mapper_args__ = {
+                "polymorphic_identity": "child1",
+            }
+
+        class Child2(Parent):
+            __tablename__ = "children_2"
+
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+
+            __mapper_args__ = {
+                "polymorphic_identity": "child2",
+            }
+
+        class Other(Base):
+            __tablename__ = "others"
+
+            id = Column(Integer, primary_key=True)
+            parent_id = Column(Integer, ForeignKey(Parent.id))
+
+            parent = relationship(Parent)
+            child2 = relationship(Child2, viewonly=True)
+
+    @classmethod
+    def insert_data(cls):
+        Other, Child1 = cls.classes("Other", "Child1")
+        s = Session()
+        obj = Other(parent=Child1())
+        s.add(obj)
+        s.commit()
+
+    def test_load_m2o_emit_query(self):
+        Other, Child1 = self.classes("Other", "Child1")
+        s = Session()
+
+        obj = s.query(Other).first()
+
+        is_(obj.child2, None)
+        eq_(obj.parent, Child1())
+
+    def test_load_m2o_use_get(self):
+        Other, Child1 = self.classes("Other", "Child1")
+        s = Session()
+
+        obj = s.query(Other).first()
+        c1 = s.query(Child1).first()
+
+        is_(obj.child2, None)
+        is_(obj.parent, c1)