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

3 files changed, 75 insertions, 14 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index a3cc96f99..be104e4f2 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -23,6 +23,18 @@
 
     .. 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
 
         Fixed bug in new :meth:`.SessionEvents.persistent_to_deleted` event
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 202b652b7..41d5dd9a3 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 9f707c791..962cb338b 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)