- 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

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index fe1795791139c542012afb752f3909cfc6cbecc0..5243b7a4dc9255da17d684c3d2d24e39355c98ef 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -22,6 +22,18 @@
     on compatibility concerns, see :doc:`/changelog/migration_10`.
 
 
+    .. 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 c3edbf6e6d9bf914a3bb9289d24eadd813ae83d8..2d8a81f0a5894954e393eac6d72ae2bc936f949b 100644 (file)
--- 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 fc8390a79ebfebcd70817963a98fe1a3307b89eb..7b3f721a61bc96f9f875ab74eb01bfd54c5efffd 100644 (file)
--- 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 16d14026c2b40356e93909e230108c14ee1421da..e99e227259311a2fef70dd0fe0147aef8215e477 100644 (file)
--- 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()
+