From c4f28097aa6a01efaa02b9792d5d15f33ae6baac Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 10 May 2017 14:03:28 -0400
Subject: [PATCH] Add conditionals specific to deferred for expire ro
 properties

Fixed bug where a :func:`.column_property` that is also marked as
"deferred" would be marked as "expired" during a flush, causing it
to be loaded along with the unexpiry of regular attributes even
though this attribute was never accessed.

Change-Id: Iaa9e17b66ece30a8e729e4af746b31ff99b1ec9a
Fixes: #3984
---
 doc/build/changelog/changelog_12.rst |  9 +++++
 lib/sqlalchemy/orm/persistence.py    | 12 +++++--
 test/orm/test_unitofwork.py          | 53 ++++++++++++++++++++++++++--
 3 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst
index b53ed4530f..24c525f850 100644
--- a/doc/build/changelog/changelog_12.rst
+++ b/doc/build/changelog/changelog_12.rst
@@ -13,6 +13,15 @@
 .. changelog::
     :version: 1.2.0b1
 
+    .. change:: 3984
+        :tags: bug, orm
+        :tickets: 3984
+
+        Fixed bug where a :func:`.column_property` that is also marked as
+        "deferred" would be marked as "expired" during a flush, causing it
+        to be loaded along with the unexpiry of regular attributes even
+        though this attribute was never accessed.
+
     .. change:: 3873
         :tags: bug, sql
         :tickets: 3873
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 5dc5a90b15..588a1d6962 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -982,8 +982,16 @@ def _finalize_insert_update_commands(base_mapper, uowtransaction, states):
 
         if mapper._readonly_props:
             readonly = state.unmodified_intersection(
-                [p.key for p in mapper._readonly_props
-                    if p.expire_on_flush or p.key not in state.dict]
+                [
+                    p.key for p in mapper._readonly_props
+                    if (
+                        p.expire_on_flush and
+                        (not p.deferred or p.key in state.dict)
+                    ) or (
+                        not p.expire_on_flush and
+                        not p.deferred and p.key not in state.dict
+                    )
+                ]
             )
             if readonly:
                 state._expire_attributes(state.dict, readonly)
diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py
index ea24d4bf01..74014781b9 100644
--- a/test/orm/test_unitofwork.py
+++ b/test/orm/test_unitofwork.py
@@ -1078,7 +1078,7 @@ class ColumnPropertyTest(fixtures.MappedTest):
         })
         self._test(True)
 
-    def test_no_refresh(self):
+    def test_no_refresh_ro_column_property_no_expire_on_flush(self):
         Data, data = self.classes.Data, self.tables.data
 
         mapper(Data, data, properties={
@@ -1088,6 +1088,36 @@ class ColumnPropertyTest(fixtures.MappedTest):
         })
         self._test(False)
 
+    def test_no_refresh_ro_column_property_expire_on_flush(self):
+        Data, data = self.classes.Data, self.tables.data
+
+        mapper(Data, data, properties={
+            'aplusb': column_property(
+                data.c.a + literal_column("' '") + data.c.b,
+                expire_on_flush=True)
+        })
+        self._test(True)
+
+    def test_no_refresh_ro_deferred_no_expire_on_flush(self):
+        Data, data = self.classes.Data, self.tables.data
+
+        mapper(Data, data, properties={
+            'aplusb': column_property(
+                data.c.a + literal_column("' '") + data.c.b,
+                expire_on_flush=False, deferred=True)
+        })
+        self._test(False, expect_deferred_load=True)
+
+    def test_no_refresh_ro_deferred_expire_on_flush(self):
+        Data, data = self.classes.Data, self.tables.data
+
+        mapper(Data, data, properties={
+            'aplusb': column_property(
+                data.c.a + literal_column("' '") + data.c.b,
+                expire_on_flush=True, deferred=True)
+        })
+        self._test(True, expect_deferred_load=True)
+
     def test_refreshes_post_init(self):
         Data, data = self.classes.Data, self.tables.data
 
@@ -1115,7 +1145,7 @@ class ColumnPropertyTest(fixtures.MappedTest):
         sess.flush()
         eq_(sd1.aplusb, "hello there")
 
-    def _test(self, expect_expiry):
+    def _test(self, expect_expiry, expect_deferred_load=False):
         Data = self.classes.Data
 
         sess = create_session()
@@ -1138,6 +1168,25 @@ class ColumnPropertyTest(fixtures.MappedTest):
         sess.flush()
         eq_(d1.aplusb, "im setting this explicitly")
 
+        # test issue #3984.
+        # NOTE: if we only expire_all() here rather than start with brand new
+        # 'd1', d1.aplusb since it was loaded moves into "expired" and stays
+        # "undeferred".  this is questionable but not as severe as the never-
+        # loaded attribute being loaded during an unexpire.
+
+        sess.close()
+        d1 = sess.query(Data).first()
+
+        d1.b = 'so long'
+        sess.flush()
+        sess.expire_all()
+        eq_(d1.b, 'so long')
+        if expect_deferred_load:
+            eq_('aplusb' in d1.__dict__, False)
+        else:
+            eq_('aplusb' in d1.__dict__, True)
+        eq_(d1.aplusb, "hello so long")
+
 
 class OneToManyTest(_fixtures.FixtureTest):
     run_inserts = None
-- 
2.39.5