Apply recursive check to immediateloader and generalize

Fixed critical regression caused by the new feature added as part of
:ticket:`1763`, eager loaders are invoked on unexpire operations. The new
feature makes use of the "immediateload" eager loader strategy as a
substitute for a collection loading strategy, which unlike the other
"post-load" strategies was not accommodating for recursive invocations
between mutually-dependent relationships, leading to recursion overflow
errors.

Fixes: #6232
Change-Id: Ifb2286281f40d1a04c24741261d4438659b6e3dd

diff --git a/doc/build/changelog/unreleased_14/6232.rst b/doc/build/changelog/unreleased_14/6232.rst
new file mode 100644 (file)
index 0000000..5aee050
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6232.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, orm, regression
+    :tickets: 6232
+
+    Fixed critical regression caused by the new feature added as part of
+    :ticket:`1763`, eager loaders are invoked on unexpire operations. The new
+    feature makes use of the "immediateload" eager loader strategy as a
+    substitute for a collection loading strategy, which unlike the other
+    "post-load" strategies was not accommodating for recursive invocations
+    between mutually-dependent relationships, leading to recursion overflow
+    errors.
+

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 824df040af531710f30a7990cf6d5077b6fad2da..da8e0439feff3c29c1cceee30b5fb138e4cdfdec 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1166,6 +1166,28 @@ class LoadLazyAttribute(object):
 class PostLoader(AbstractRelationshipLoader):
     """A relationship loader that emits a second SELECT statement."""
 
+    def _check_recursive_postload(self, context, path, join_depth=None):
+        effective_path = (
+            context.compile_state.current_path or orm_util.PathRegistry.root
+        ) + path
+
+        if loading.PostLoad.path_exists(
+            context, effective_path, self.parent_property
+        ):
+            return True
+
+        path_w_prop = path[self.parent_property]
+        effective_path_w_prop = effective_path[self.parent_property]
+
+        if not path_w_prop.contains(context.attributes, "loader"):
+            if join_depth:
+                if effective_path_w_prop.length / 2 > join_depth:
+                    return True
+            elif effective_path_w_prop.contains_mapper(self.mapper):
+                return True
+
+        return False
+
     def _immediateload_create_row_processor(
         self,
         context,
@@ -1214,6 +1236,9 @@ class ImmediateLoader(PostLoader):
         def load_immediate(state, dict_, row):
             state.get_impl(self.key).get(state, dict_)
 
+        if self._check_recursive_postload(context, path):
+            return
+
         populators["delayed"].append((self.key, load_immediate))
 
 
@@ -1727,6 +1752,7 @@ class SubqueryLoader(PostLoader):
         adapter,
         populators,
     ):
+
         if context.refresh_state:
             return self._immediateload_create_row_processor(
                 context,
@@ -1738,6 +1764,10 @@ class SubqueryLoader(PostLoader):
                 adapter,
                 populators,
             )
+        # the subqueryloader does a similar check in setup_query() unlike
+        # the other post loaders, however we have this here for consistency
+        elif self._check_recursive_postload(context, path, self.join_depth):
+            return
 
         if not self.parent.class_manager[self.key].impl.supports_population:
             raise sa_exc.InvalidRequestError(
@@ -2689,6 +2719,7 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
         adapter,
         populators,
     ):
+
         if context.refresh_state:
             return self._immediateload_create_row_processor(
                 context,
@@ -2700,6 +2731,8 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
                 adapter,
                 populators,
             )
+        elif self._check_recursive_postload(context, path, self.join_depth):
+            return
 
         if not self.parent.class_manager[self.key].impl.supports_population:
             raise sa_exc.InvalidRequestError(
@@ -2721,13 +2754,7 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
             context.compile_state.current_path or orm_util.PathRegistry.root
         ) + path
 
-        if loading.PostLoad.path_exists(
-            context, selectin_path, self.parent_property
-        ):
-            return
-
         path_w_prop = path[self.parent_property]
-        selectin_path_w_prop = selectin_path[self.parent_property]
 
         # build up a path indicating the path from the leftmost
         # entity to the thing we're subquery loading.
@@ -2739,12 +2766,6 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
         else:
             effective_entity = self.entity
 
-        if not path_w_prop.contains(context.attributes, "loader"):
-            if self.join_depth:
-                if selectin_path_w_prop.length / 2 > self.join_depth:
-                    return
-            elif selectin_path_w_prop.contains_mapper(self.mapper):
-                return
         loading.PostLoad.callable_for_path(
             context,
             selectin_path,

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index c0213cf2271d320481c0764bc8ae66ae4acf11f6..421568b65aaae6f617aee826a52de9609c5548dd 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -3703,6 +3703,44 @@ class LoadOnExistingTest(_fixtures.FixtureTest):
 
         assert "addresses" in u1.__dict__
 
+    @testing.combinations(
+        ("selectin",),
+        ("subquery",),
+        ("immediate",),
+    )
+    def test_refresh_no_recursion(self, strat):
+        User, Address = self.classes.User, self.classes.Address
+        mapper(
+            User,
+            self.tables.users,
+            properties={
+                "addresses": relationship(
+                    Address, lazy="joined", back_populates="user"
+                )
+            },
+        )
+        mapper(
+            Address,
+            self.tables.addresses,
+            properties={
+                "user": relationship(
+                    User, lazy=strat, back_populates="addresses"
+                )
+            },
+        )
+        sess = fixture_session(autoflush=False)
+
+        u1 = sess.query(User).get(8)
+        assert "addresses" in u1.__dict__
+        sess.expire(u1)
+
+        def go():
+            eq_(u1.id, 8)
+
+        self.assert_sql_count(testing.db, go, 1)
+
+        assert "addresses" in u1.__dict__
+
     def test_populate_existing_propagate(self):
         # both SelectInLoader and SubqueryLoader receive the loaded collection
         # at once and use attributes.set_committed_value().  However