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

diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst
index b53ed4530fcb5411c6779f02a8c8b0d9c81e30ed..24c525f8508d896ee62b7eeaacc2535f8db8f749 100644 (file)
--- 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 5dc5a90b158a8bcf359a8320680971741c64f477..588a1d6962dfd2579533f79aeb88f4ac5407df78 100644 (file)
--- 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 ea24d4bf01daedc527f26dc6a2d0111bcfb9813a..74014781b9f1147feed87496cb2509411f473a4c 100644 (file)
--- 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