From c3abfe50645abdb27e41639701a68d2e6eaeff2c Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 3 Oct 2016 16:55:54 -0400
Subject: [PATCH] Honor additional row coming in with value of None

The change in #3431 still checks that the instance() is
non-None, deferring to other loading schemes if it is.
These columns are dedicated towards the entity however, so if the value
is None, we should set it.  If it conflicts, we are detecting that
in any case.

Change-Id: I223768e2898e843f953e910da1f9564b137d95e4
Fixes: #3811
---
 doc/build/changelog/changelog_11.rst | 12 +++++++
 lib/sqlalchemy/orm/strategies.py     | 26 +++++++-------
 test/orm/test_eager_relations.py     | 51 +++++++++++++++++++++++++++-
 3 files changed, 75 insertions(+), 14 deletions(-)

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index a3cc96f992..be104e4f28 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -21,6 +21,18 @@
 .. changelog::
     :version: 1.1.0
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3811
+
+        Made an adjustment to the bug fix first introduced in [ticket:3431]
+        that involves an object appearing in multiple contexts in a single
+        result set, such that an eager loader that would set the related
+        object value to be None will still fire off, thus satisfying the
+        load of that attribute.  Previously, the adjustment only honored
+        a non-None value arriving for an eagerly loaded attribute in a
+        secondary row.
+
     .. change::
         :tags: bug, orm
         :tickets: 3808
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 202b652b72..41d5dd9a32 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1616,19 +1616,19 @@ class JoinedLoader(AbstractRelationshipLoader):
             # call _instance on the row, even though the object has
             # been created, so that we further descend into properties
             existing = _instance(row)
-            if existing is not None:
-                # conflicting value already loaded, this shouldn't happen
-                if key in dict_:
-                    if existing is not dict_[key]:
-                        util.warn(
-                            "Multiple rows returned with "
-                            "uselist=False for eagerly-loaded attribute '%s' "
-                            % self)
-                else:
-                    # this case is when one row has multiple loads of the
-                    # same entity (e.g. via aliasing), one has an attribute
-                    # that the other doesn't.
-                    dict_[key] = existing
+
+            # conflicting value already loaded, this shouldn't happen
+            if key in dict_:
+                if existing is not dict_[key]:
+                    util.warn(
+                        "Multiple rows returned with "
+                        "uselist=False for eagerly-loaded attribute '%s' "
+                        % self)
+            else:
+                # this case is when one row has multiple loads of the
+                # same entity (e.g. via aliasing), one has an attribute
+                # that the other doesn't.
+                dict_[key] = existing
 
         def load_scalar_from_joined_exec(state, dict_, row):
             _instance(row)
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 9f707c791e..962cb338b5 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -4252,7 +4252,6 @@ class EntityViaMultiplePathTestOne(fixtures.DeclarativeMappedTest):
         # PYTHONHASHSEED
         in_('d', a1.c.__dict__)
 
-
 class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest):
     """test for [ticket:3431]"""
 
@@ -4324,3 +4323,53 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest):
         in_(
             'user', lz_test.a.ld.__dict__
         )
+
+
+class EntityViaMultiplePathTestThree(fixtures.DeclarativeMappedTest):
+    """test for [ticket:3811] continuing on [ticket:3431]"""
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class A(Base):
+            __tablename__ = 'a'
+
+            id = Column(Integer, primary_key=True)
+            parent_id = Column(Integer, ForeignKey('a.id'))
+            parent = relationship("A", remote_side=id, lazy="raise")
+
+    def test_multi_path_load_lazy_none(self):
+        A = self.classes.A
+        s = Session()
+        s.add_all([
+            A(id=1, parent_id=None),
+            A(id=2, parent_id=2),
+            A(id=4, parent_id=None),
+            A(id=3, parent_id=4),
+        ])
+        s.commit()
+
+        q1 = s.query(A).order_by(A.id).\
+            filter(A.id.in_([1, 2])).options(joinedload(A.parent))
+
+        def go():
+            for a in q1:
+                if a.id == 1:
+                    assert a.parent is None
+                else:
+                    assert a.parent is not None
+
+        self.assert_sql_count(testing.db, go, 1)
+
+        q1 = s.query(A).order_by(A.id).\
+            filter(A.id.in_([3, 4])).options(joinedload(A.parent))
+
+        def go():
+            for a in q1:
+                if a.id == 4:
+                    assert a.parent is None
+                else:
+                    assert a.parent is not None
+
+        self.assert_sql_count(testing.db, go, 1)
-- 
2.47.2