- Made a small adjustment to the mechanics of lazy loading,

such that it has less chance of interfering with a joinload() in the
very rare circumstance that an object points to itself; in this
scenario, the object refers to itself while loading its attributes
which can cause a mixup between loaders.   The use case of
"object points to itself" is not fully supported, but the fix also
removes some overhead so for now is part of testing.
fixes #3145

4 files changed, 98 insertions, 3 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index fe1795791..5243b7a4d 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -23,6 +23,18 @@
 
 
     .. change::
+        :tags: bug, orm
+        :tickets: 3145
+
+        Made a small adjustment to the mechanics of lazy loading,
+        such that it has less chance of interfering with a joinload() in the
+        very rare circumstance that an object points to itself; in this
+        scenario, the object refers to itself while loading its attributes
+        which can cause a mixup between loaders.   The use case of
+        "object points to itself" is not fully supported, but the fix also
+        removes some overhead so for now is part of testing.
+
+    .. change::
         :tags: feature, orm
         :tickets: 3176
 
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index c3edbf6e6..2d8a81f0a 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -634,7 +634,7 @@ class LazyLoader(AbstractRelationshipLoader):
                 LoadLazyAttribute(key), key)
 
             return set_lazy_callable, None, None
-        else:
+        elif context.populate_existing or mapper.always_refresh:
             def reset_for_lazy_callable(state, dict_, row):
                 # we are the primary manager for this attribute on
                 # this class - reset its
@@ -647,6 +647,9 @@ class LazyLoader(AbstractRelationshipLoader):
                 state._reset(dict_, key)
 
             return reset_for_lazy_callable, None, None
+        else:
+            return None, None, None
+
 
 
 class LoadLazyAttribute(object):
diff --git a/lib/sqlalchemy/testing/util.py b/lib/sqlalchemy/testing/util.py
index fc8390a79..7b3f721a6 100644
--- a/lib/sqlalchemy/testing/util.py
+++ b/lib/sqlalchemy/testing/util.py
@@ -203,5 +203,7 @@ class adict(dict):
         except KeyError:
             return dict.__getattribute__(self, key)
 
-    def get_all(self, *keys):
+    def __call__(self, *keys):
         return tuple([self[key] for key in keys])
+
+    get_all = __call__
diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py
index 16d14026c..e99e22725 100644
--- a/test/orm/test_lazy_relations.py
+++ b/test/orm/test_lazy_relations.py
@@ -2,7 +2,7 @@
 
 from sqlalchemy.testing import assert_raises
 import datetime
-from sqlalchemy.orm import attributes, exc as orm_exc
+from sqlalchemy.orm import attributes, exc as orm_exc, configure_mappers
 import sqlalchemy as sa
 from sqlalchemy import testing, and_
 from sqlalchemy import Integer, String, ForeignKey, SmallInteger, Boolean
@@ -892,3 +892,81 @@ class O2MWOSideFixedTest(fixtures.MappedTest):
             [p.id for p in c2.people],
             []
         )
+
+
+class RefersToSelfLazyLoadInterferenceTest(fixtures.MappedTest):
+    """Test [issue:3145].
+
+    This involves an object that refers to itself, which isn't
+    entirely a supported use case.   Here, we're able to fix it,
+    but long term it's not clear if future needs will affect this.
+    The use case is not super-critical.
+
+    """
+
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'a', metadata,
+            Column('a_id', Integer, primary_key=True),
+            Column('b_id', ForeignKey('b.b_id')),
+        )
+
+        Table(
+            'b', metadata,
+            Column('b_id', Integer, primary_key=True),
+            Column('parent_id', ForeignKey('b.b_id')),
+        )
+
+        Table(
+            'c', metadata,
+            Column('c_id', Integer, primary_key=True),
+            Column('b_id', ForeignKey('b.b_id')),
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class A(cls.Basic):
+            pass
+
+        class B(cls.Basic):
+            pass
+
+        class C(cls.Basic):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        mapper(cls.classes.A, cls.tables.a, properties={
+            "b": relationship(cls.classes.B)
+        })
+        bm = mapper(cls.classes.B, cls.tables.b, properties={
+            "parent": relationship(
+                cls.classes.B, remote_side=cls.tables.b.c.b_id),
+            "zc": relationship(cls.classes.C)
+        })
+        mapper(cls.classes.C, cls.tables.c)
+
+        bmp = bm._props
+        configure_mappers()
+        # Bug is order-dependent, must sort the "zc" property to the end
+        bmp.sort()
+
+    def test_lazy_doesnt_interfere(self):
+        A, B, C = self.classes("A", "B", "C")
+
+        session = Session()
+        b = B()
+        session.add(b)
+        session.flush()
+
+        b.parent_id = b.b_id
+
+        b.zc.append(C())
+        b.zc.append(C())
+        session.commit()
+
+        # If the bug is here, the next line throws an exception
+        session.query(B).options(
+            sa.orm.joinedload('parent').joinedload('zc')).all()
+