From 3e58122bdd815b5677b35efc176eda1625e8b37e Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 2 Sep 2020 14:47:03 -0400
Subject: [PATCH] Add caveat re: with_expression and already-loaded object

Adds a test for the populate_existing() behavior as well.

Fixes: #5553
Change-Id: Ib0db6227c3fec7d0065f2a7caa36b3fd94ef14fd
(cherry picked from commit b050169600019ef249a8b315e7435c752623c900)
---
 doc/build/orm/mapped_sql_expr.rst      | 15 +++++++++++-
 lib/sqlalchemy/orm/strategy_options.py |  6 +++++
 test/orm/test_deferred.py              | 32 ++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/doc/build/orm/mapped_sql_expr.rst b/doc/build/orm/mapped_sql_expr.rst
index f7ee2020ec..56c7d8f45c 100644
--- a/doc/build/orm/mapped_sql_expr.rst
+++ b/doc/build/orm/mapped_sql_expr.rst
@@ -289,7 +289,20 @@ The :func:`.query_expression` mapping has these caveats:
 
 * On an object where :func:`.query_expression` were not used to populate
   the attribute, the attribute on an object instance will have the value
-  ``None``.
+  ``None``, unless the :paramref:`_orm.query_expression.default_expr`
+  parameter is set to an alternate SQL expression.
+
+* The query_expression value **does not populate on an object that is
+  already loaded**.  That is, this will **not work**::
+
+    obj = session.query(A).first()
+
+    obj = session.query(A).options(with_expression(A.expr, some_expr)).first()
+
+  To ensure the attribute is re-loaded, use :meth:`_orm.Query.populate_existing`::
+
+    obj = session.query(A).populate_existing().options(
+        with_expression(A.expr, some_expr)).first()
 
 * The query_expression value **does not refresh when the object is
   expired**.  Once the object is expired, either via :meth:`.Session.expire`
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index cba0e7e80f..d5404b550f 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1704,6 +1704,12 @@ def with_expression(loadopt, key, expression):
 
     :param expr: SQL expression to be applied to the attribute.
 
+    .. note:: the target attribute is populated only if the target object
+       is **not currently loaded** in the current :class:`_orm.Session`
+       unless the :meth:`_orm.Query.populate_existing` method is used.
+       Please refer to :ref:`mapper_querytime_expression` for complete
+       usage details.
+
     .. seealso::
 
         :ref:`mapper_querytime_expression`
diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py
index b4f848a968..28d771234e 100644
--- a/test/orm/test_deferred.py
+++ b/test/orm/test_deferred.py
@@ -1742,6 +1742,38 @@ class WithExpressionTest(fixtures.DeclarativeMappedTest):
             q.all(), [A(bs=[B(b_expr=25)]), A(bs=[B(b_expr=38), B(b_expr=10)])]
         )
 
+    def test_no_refresh_unless_populate_existing(self):
+        A = self.classes.A
+
+        s = Session()
+        a1 = s.query(A).first()
+
+        def go():
+            eq_(a1.my_expr, None)
+
+        self.assert_sql_count(testing.db, go, 0)
+
+        a1 = s.query(A).options(with_expression(A.my_expr, A.x + A.y)).first()
+
+        eq_(a1.my_expr, None)
+
+        a1 = (
+            s.query(A)
+            .populate_existing()
+            .options(with_expression(A.my_expr, A.x + A.y))
+            .first()
+        )
+
+        eq_(a1.my_expr, 3)
+
+        a1 = s.query(A).first()
+
+        eq_(a1.my_expr, 3)
+
+        s.expire(a1)
+
+        eq_(a1.my_expr, None)
+
     def test_no_sql_not_set_up(self):
         A = self.classes.A
 
-- 
2.47.3