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

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index a3cc96f99291bcf8e6ed0ed0b94df8f162cec477..be104e4f2804ca0d17148c4010309e6e5dd4ceff 100644 (file)
--- 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 202b652b727c6ad462fa8b19ee109b11026e3f3f..41d5dd9a320505389ae7d8a0ff5a60838d8a5dac 100644 (file)
--- 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 9f707c791e334f4e9f5f5d204f89c98b3c3f2b61..962cb338b5d2b08cb9748a2b77af65f398f892e3 100644 (file)
--- 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)