From dd6f4543bc8ccbf07bfc5c8fb850be60ab420b57 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 4 May 2021 11:19:00 -0400
Subject: [PATCH] Restore detached object logic for dynamic, but warn

Fixed regression involving ``lazy='dynamic'`` loader in conjunction with a
detached object. The previous behavior was that the dynamic loader upon
calling methods like ``.all()`` returns empty lists for detached objects
without error, this has been restored; however a warning is now emitted as
this is not the correct result. Other dynamic loader scenarios correctly
raise ``DetachedInstanceError``.

Fixes: #6426
Change-Id: Id7ad204bef947491fa7e462c5acda2055fada910
---
 doc/build/changelog/unreleased_14/6426.rst | 10 +++++++
 lib/sqlalchemy/engine/result.py            |  9 ++++++-
 lib/sqlalchemy/orm/dynamic.py              | 22 +++++++++++----
 test/orm/test_dynamic.py                   | 31 ++++++++++++++++++++++
 4 files changed, 66 insertions(+), 6 deletions(-)
 create mode 100644 doc/build/changelog/unreleased_14/6426.rst

diff --git a/doc/build/changelog/unreleased_14/6426.rst b/doc/build/changelog/unreleased_14/6426.rst
new file mode 100644
index 0000000000..d0a3cc28ea
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6426.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, regression, orm
+    :tickets: 6426
+
+    Fixed regression involving ``lazy='dynamic'`` loader in conjunction with a
+    detached object. The previous behavior was that the dynamic loader upon
+    calling methods like ``.all()`` returns empty lists for detached objects
+    without error, this has been restored; however a warning is now emitted as
+    this is not the correct result. Other dynamic loader scenarios correctly
+    raise ``DetachedInstanceError``.
diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py
index f02ceff156..a803959419 100644
--- a/lib/sqlalchemy/engine/result.py
+++ b/lib/sqlalchemy/engine/result.py
@@ -1593,10 +1593,17 @@ class IteratorResult(Result):
 
     """
 
-    def __init__(self, cursor_metadata, iterator, raw=None):
+    def __init__(
+        self,
+        cursor_metadata,
+        iterator,
+        raw=None,
+        _source_supports_scalars=False,
+    ):
         self._metadata = cursor_metadata
         self.iterator = iterator
         self.raw = raw
+        self._source_supports_scalars = _source_supports_scalars
 
     def _soft_close(self, **kw):
         self.iterator = iter([])
diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index a4b5f58c72..ac7eba03b8 100644
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -24,6 +24,7 @@ from .query import Query
 from .. import exc
 from .. import log
 from .. import util
+from ..engine import result
 
 
 @log.class_logger
@@ -324,17 +325,28 @@ class AppenderMixin(object):
 
     session = property(session, lambda s, x: None)
 
-    def __iter__(self):
+    def _iter(self):
         sess = self.session
         if sess is None:
-            return iter(
+            state = attributes.instance_state(self.instance)
+            if state.detached:
+                util.warn(
+                    "Instance %s is detached, dynamic relationship cannot "
+                    "return a correct result.   This warning will become "
+                    "a DetachedInstanceError in a future release."
+                    % (orm_util.state_str(state))
+                )
+
+            return result.IteratorResult(
+                result.SimpleResultMetaData([self.attr.class_.__name__]),
                 self.attr._get_collection_history(
                     attributes.instance_state(self.instance),
                     attributes.PASSIVE_NO_INITIALIZE,
-                ).added_items
-            )
+                ).added_items,
+                _source_supports_scalars=True,
+            ).scalars()
         else:
-            return iter(self._generate(sess))
+            return self._generate(sess)._iter()
 
     def __getitem__(self, index):
         sess = self.session
diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py
index 8ea04c2689..e87b5a3636 100644
--- a/test/orm/test_dynamic.py
+++ b/test/orm/test_dynamic.py
@@ -233,6 +233,8 @@ class DynamicTest(_DynamicFixture, _fixtures.FixtureTest, AssertsCompiledSQL):
         )
 
     def test_detached_raise(self):
+        """so filtering on a detached dynamic list raises an error..."""
+
         User, Address = self._user_address_fixture()
         sess = fixture_session()
         u = sess.query(User).get(8)
@@ -243,6 +245,35 @@ class DynamicTest(_DynamicFixture, _fixtures.FixtureTest, AssertsCompiledSQL):
             email_address="e",
         )
 
+    def test_detached_all_empty_list(self):
+        """test #6426 - but you can call .all() on it and you get an empty
+        list.   This is legacy stuff, as this should be raising
+        DetachedInstanceError.
+
+        """
+
+        User, Address = self._user_address_fixture()
+        sess = fixture_session()
+        u = sess.query(User).get(8)
+        sess.expunge(u)
+
+        with testing.expect_warnings(
+            r"Instance <User .*> is detached, dynamic relationship"
+        ):
+            eq_(u.addresses.all(), [])
+
+        with testing.expect_warnings(
+            r"Instance <User .*> is detached, dynamic relationship"
+        ):
+            eq_(list(u.addresses), [])
+
+    def test_transient_all_empty_list(self):
+        User, Address = self._user_address_fixture()
+        u1 = User()
+        eq_(u1.addresses.all(), [])
+
+        eq_(list(u1.addresses), [])
+
     def test_no_uselist_false(self):
         User, Address = self._user_address_fixture(
             addresses_args={"uselist": False}
-- 
2.47.3