From: Mike Bayer Date: Sat, 21 Mar 2020 21:26:24 +0000 (-0400) Subject: Test instance for matching class hierarchy on get_from_identity X-Git-Tag: rel_1_3_16~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=84c89f781a8abb608d9e4cb0d9e00e2221fcc093;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..0a50ba04e9 --- /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 f7416efb88..a8e590cf4f 100644 --- 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 fab43e4cff..7d28e1082e 100644 --- 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 23c0c4e49b..06224b7f89 100644 --- 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 478e70283c..3e975890e4 100644 --- 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 4bd74988a5..22976321aa 100644 --- 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 51eddee719..851ad60b87 100644 --- 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)