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

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 a959b0a4008da9efd80d3c86f3d6c6c04d957de0..5ca2858e905794490bcfb61897682c321975bb89 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 a31745aec278f3cfa0254ce4adf255aa0bed9c3e..6df8acf1ee9a21ba9dc3fec66b10630dfc6b24ee 100644 (file)
--- a/lib/sqlalchemy/orm/base.py
+++ b/lib/sqlalchemy/orm/base.py
@@ -25,6 +25,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 c3d4773cd3ba3b18a164122fec3c4990c80175bd..49c71e5b2c8c15fc0d60ee6566e7e63fe3352965 100644 (file)
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -161,7 +161,7 @@ def merge_result(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.
 
@@ -171,6 +171,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 fe1ea9bfcdd668ba4d0e48bce96eb5cbea1e2857..0da7d08a4aece64bc28e96f78fb9bb3a688f57ee 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -1048,6 +1048,8 @@ class Query(Generative):
                 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/session.py b/lib/sqlalchemy/orm/session.py
index f172649ba2c0aa371f2e475fa63f8da11b262e1d..fefdd4ef1ad1646319763297058ee43a92a448d8 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1617,7 +1617,7 @@ class Session(_SessionClassMethods):
         key = mapper.identity_key_from_primary_key(
             primary_key_identity, identity_token=identity_token
         )
-        return loading.get_from_identity(self, key, passive)
+        return loading.get_from_identity(self, mapper, key, passive)
 
     @property
     @util.contextmanager

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 7d05a37f5c73751724b10563fe3f4908cb3b840f..094637082158eab3978c9c7e7a6390c5826d0ceb 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -774,7 +774,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 9bbee1e2c8abd7b97ccc39f67b452603d93216cd..b7d49829e01e061922eb03474350b032bf9af8eb 100644 (file)
--- a/test/orm/inheritance/test_relationship.py
+++ b/test/orm/inheritance/test_relationship.py
@@ -19,6 +19,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
 
@@ -2682,3 +2683,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)