Don't populate expired attrs w/ evaluator

Fixed bug in :meth:`_orm.Query.update` where objects in the
:class:`_orm.Session` that were already expired would be unnecessarily
SELECTed individually when they were refreshed by the "evaluate"
synchronize strategy.

For 1.4 there was also a similar issue with fetch that would actually
get the wrong data back, as the new value would be loaded, then
applied with the evaluator.

Fixes: #5664
Change-Id: I6e6aff88462654fcefa9fce2b45f5446c418deee
(cherry picked from commit e3dc20ff27fa75e571fb2631e64737ea8f25f7c5)

diff --git a/doc/build/changelog/unreleased_13/5664.rst b/doc/build/changelog/unreleased_13/5664.rst
new file mode 100644 (file)
index 0000000..9c79697
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5664.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 5664
+
+    Fixed bug in :meth:`_orm.Query.update` where objects in the
+    :class:`_orm.Session` that were already expired would be unnecessarily
+    SELECTed individually when they were refreshed by the "evaluate"
+    synchronize strategy.

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 7ec1e8c53aac2b237c742d10070521fb728fa7fe..93460da4933a651c98dfa9ece2467401bc4d31d3 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -1957,6 +1957,8 @@ class BulkUpdateEvaluate(BulkEvaluate, BulkUpdate):
             # only evaluate unmodified attributes
             to_evaluate = state.unmodified.intersection(evaluated_keys)
             for key in to_evaluate:
+                if key not in dict_:
+                    continue
                 dict_[key] = self.value_evaluators[key](obj)
 
             state.manager.dispatch.refresh(state, None, to_evaluate)

diff --git a/test/orm/test_update_delete.py b/test/orm/test_update_delete.py
index 46bb870621d5bb38d8f46d2f438a9a4fb3e551f5..b715991dabc6f975ba87b8d71602aa83a322dbe7 100644 (file)
--- a/test/orm/test_update_delete.py
+++ b/test/orm/test_update_delete.py
@@ -22,6 +22,7 @@ from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
 from sqlalchemy.testing import mock
+from sqlalchemy.testing.assertsql import CompiledSQL
 from sqlalchemy.testing.schema import Column
 from sqlalchemy.testing.schema import Table
 
@@ -229,6 +230,114 @@ class UpdateDeleteTest(fixtures.MappedTest):
         )
         eq_(jill.ufoo, "moonbeam")
 
+    def test_evaluate_dont_refresh_expired_objects(self):
+        User = self.classes.User
+
+        sess = Session()
+
+        john, jack, jill, jane = sess.query(User).order_by(User.id).all()
+
+        sess.expire(john)
+        sess.expire(jill)
+        sess.expire(jane, ["name"])
+
+        with self.sql_execution_asserter() as asserter:
+            # using 1.x style for easier backport
+            sess.query(User).update(
+                {"age": User.age + 10}, synchronize_session="evaluate"
+            )
+
+        asserter.assert_(
+            CompiledSQL(
+                "UPDATE users SET age_int=(users.age_int + :age_int_1)",
+                [{"age_int_1": 10}],
+            ),
+        )
+
+        with self.sql_execution_asserter() as asserter:
+            eq_(john.age, 35)  # needs refresh
+            eq_(jack.age, 57)  # no SQL needed
+            eq_(jill.age, 39)  # needs refresh
+            eq_(jane.age, 47)  # no SQL needed
+
+        asserter.assert_(
+            # refresh john
+            CompiledSQL(
+                "SELECT users.age_int AS users_age_int, "
+                "users.id AS users_id, users.name AS users_name FROM users "
+                "WHERE users.id = :param_1",
+                [{"param_1": 1}],
+            ),
+            # refresh jill
+            CompiledSQL(
+                "SELECT users.age_int AS users_age_int, "
+                "users.id AS users_id, users.name AS users_name FROM users "
+                "WHERE users.id = :param_1",
+                [{"param_1": 3}],
+            ),
+        )
+
+    def test_fetch_dont_refresh_expired_objects(self):
+        User = self.classes.User
+
+        sess = Session()
+
+        john, jack, jill, jane = sess.query(User).order_by(User.id).all()
+
+        sess.expire(john)
+        sess.expire(jill)
+        sess.expire(jane, ["name"])
+
+        with self.sql_execution_asserter() as asserter:
+            # using 1.x style for easier backport
+            sess.query(User).update(
+                {"age": User.age + 10}, synchronize_session="fetch"
+            )
+
+        asserter.assert_(
+            CompiledSQL("SELECT users.id AS users_id FROM users"),
+            CompiledSQL(
+                "UPDATE users SET age_int=(users.age_int + :age_int_1)",
+                [{"age_int_1": 10}],
+            ),
+        )
+
+        with self.sql_execution_asserter() as asserter:
+            eq_(john.age, 35)  # needs refresh
+            eq_(jack.age, 57)  # refreshes in 1.3
+            eq_(jill.age, 39)  # needs refresh
+            eq_(jane.age, 47)  # refreshes in 1.3
+
+        asserter.assert_(
+            # refresh john
+            CompiledSQL(
+                "SELECT users.age_int AS users_age_int, "
+                "users.id AS users_id, users.name AS users_name FROM users "
+                "WHERE users.id = :param_1",
+                [{"param_1": 1}],
+            ),
+            # refresh jack.age in 1.3 only
+            CompiledSQL(
+                "SELECT users.age_int AS users_age_int FROM users "
+                "WHERE users.id = :param_1",
+                [{"param_1": 2}],
+            ),
+            # refresh jill
+            CompiledSQL(
+                "SELECT users.age_int AS users_age_int, "
+                "users.id AS users_id, users.name AS users_name FROM users "
+                "WHERE users.id = :param_1",
+                [{"param_1": 3}],
+            ),
+            # refresh jane, seems to be a full refresh in 1.3.
+            CompiledSQL(
+                "SELECT users.age_int AS users_age_int, "
+                "users.name AS users_name FROM users "
+                "WHERE users.id = :param_1",
+                [{"param_1": 4}],
+            ),
+        )
+
     def test_delete(self):
         User = self.classes.User