Ensure states with null m2o FK value are still populated by selectinloader

Fixed regression in selectinload loader strategy caused by #4775 (released
in version 1.3.6) where a many-to-one attribute of None would no longer be
populated by the loader.  While this was usually not noticeable due to the
lazyloader populating None upon get, it would lead to a detached instance
error if the object were detached.

Fixes: #4872
Change-Id: I18466841683111643810ebc4331376df38c4767b

1 files changed, 26 insertions, 3 deletions

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index cdc5c10c9..1f2f65728 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -546,6 +546,11 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
 
         # determine if our "lazywhere" clause is the same as the mapper's
         # get() clause.  then we can just use mapper.get()
+        #
+        # TODO: the "not self.uselist" can be taken out entirely; a m2o
+        # load that populates for a list (very unusual, but is possible with
+        # the API) can still set for "None" and the attribute system will
+        # populate as an empty list.
         self.use_get = (
             not self.is_aliased_class
             and not self.uselist
@@ -2269,6 +2274,7 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
 
         if query_info.load_only_child:
             our_states = collections.defaultdict(list)
+            none_states = []
 
             mapper = self.parent
 
@@ -2289,11 +2295,19 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
                 if attributes.PASSIVE_NO_RESULT in related_ident:
                     query_info = self._fallback_query_info
                     break
+
+                # organize states into lists keyed to particular foreign
+                # key values.
                 if None not in related_ident:
                     our_states[related_ident].append(
                         (state, state_dict, overwrite)
                     )
+                else:
+                    # For FK values that have None, add them to a
+                    # separate collection that will be populated separately
+                    none_states.append((state, state_dict, overwrite))
 
+        # note the above conditional may have changed query_info
         if not query_info.load_only_child:
             our_states = [
                 (state.key[1], state, state.dict, overwrite)
@@ -2389,11 +2403,13 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
                 q.add_criteria(_setup_outermost_orderby)
 
         if query_info.load_only_child:
-            self._load_via_child(our_states, query_info, q, context)
+            self._load_via_child(
+                our_states, none_states, query_info, q, context
+            )
         else:
             self._load_via_parent(our_states, query_info, q, context)
 
-    def _load_via_child(self, our_states, query_info, q, context):
+    def _load_via_child(self, our_states, none_states, query_info, q, context):
         uselist = self.uselist
 
         # this sort is really for the benefit of the unit tests
@@ -2401,7 +2417,6 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
         while our_keys:
             chunk = our_keys[0 : self._chunksize]
             our_keys = our_keys[self._chunksize :]
-
             data = {
                 k: v
                 for k, v in q(context.session).params(
@@ -2427,6 +2442,14 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
                         dict_,
                         related_obj if not uselist else [related_obj],
                     )
+        # populate none states with empty value / collection
+        for state, dict_, overwrite in none_states:
+            if not overwrite and self.key in dict_:
+                continue
+
+            # note it's OK if this is a uselist=True attribute, the empty
+            # collection will be populated
+            state.get_impl(self.key).set_committed_value(state, dict_, None)
 
     def _load_via_parent(self, our_states, query_info, q, context):
         uselist = self.uselist