From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 2 Sep 2020 18:47:03 +0000 (-0400)
Subject: Add caveat re: with_expression and already-loaded object
X-Git-Tag: rel_1_4_0b1~130
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b050169600019ef249a8b315e7435c752623c900;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Add caveat re: with_expression and already-loaded object

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

Fixes: #5553
Change-Id: Ib0db6227c3fec7d0065f2a7caa36b3fd94ef14fd
---

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 6c6f0307d8..53350cb8e4 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1682,6 +1682,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 726f81330d..e3cefe9b76 100644
--- a/test/orm/test_deferred.py
+++ b/test/orm/test_deferred.py
@@ -1815,6 +1815,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