From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 28 Dec 2023 21:02:48 +0000 (-0500)
Subject: pop prefetch values from committed_state when they are available
X-Git-Tag: rel_2_0_25~15^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=429cbfe64c493c1cef532da892baf6871a6b2f90;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

pop prefetch values from committed_state when they are available

Fixed issue where when making use of the
:paramref:`_orm.relationship.post_update` feature at the same time as using
a mapper version_id_col could lead to a situation where the second UPDATE
statement emitted by the post-update feature would fail to make use of the
correct version identifier, assuming an UPDATE was already emitted in that
flush which had already bumped the version counter.

Fixes: #10800
Change-Id: I3fccdb26ebbd2d987bb4f0e894449b7413556054
(cherry picked from commit 46ec57e5cc5c66616087453a090754f4d0853c0c)
---

diff --git a/doc/build/changelog/unreleased_20/10800.rst b/doc/build/changelog/unreleased_20/10800.rst
new file mode 100644
index 0000000000..346ae1f5ac
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/10800.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 10800
+
+    Fixed issue where when making use of the
+    :paramref:`_orm.relationship.post_update` feature at the same time as using
+    a mapper version_id_col could lead to a situation where the second UPDATE
+    statement emitted by the post-update feature would fail to make use of the
+    correct version identifier, assuming an UPDATE was already emitted in that
+    flush which had already bumped the version counter.
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 3f537fb761..1728b4ac88 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -1659,9 +1659,18 @@ def _postfetch(
 
     for c in prefetch_cols:
         if c.key in params and c in mapper._columntoproperty:
-            dict_[mapper._columntoproperty[c].key] = params[c.key]
+            pkey = mapper._columntoproperty[c].key
+
+            # set prefetched value in dict and also pop from committed_state,
+            # since this is new database state that replaces whatever might
+            # have previously been fetched (see #10800).  this is essentially a
+            # shorthand version of set_committed_value(), which could also be
+            # used here directly (with more overhead)
+            dict_[pkey] = params[c.key]
+            state.committed_state.pop(pkey, None)
+
             if refresh_flush:
-                load_evt_attrs.append(mapper._columntoproperty[c].key)
+                load_evt_attrs.append(pkey)
 
     if refresh_flush and load_evt_attrs:
         mapper.class_manager.dispatch.refresh_flush(
diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py
index 7f52af7156..a0325059a8 100644
--- a/test/orm/test_versioning.py
+++ b/test/orm/test_versioning.py
@@ -2029,3 +2029,89 @@ class QuotedBindVersioningTest(fixtures.MappedTest):
             fixture_session.commit()
 
         eq_(f1.version, 2)
+
+
+class PostUpdateVersioningTest(fixtures.DeclarativeMappedTest):
+    """test for #10800"""
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class User(Base):
+            __tablename__ = "user"
+
+            id = Column(Integer, primary_key=True)
+
+        class Parent(Base):
+            __tablename__ = "parent"
+
+            id = Column(Integer, primary_key=True)
+            version_id = Column(Integer)
+            updated_by_id = Column(
+                Integer,
+                ForeignKey("user.id"),
+            )
+
+            updated_by = relationship(
+                "User",
+                foreign_keys=[updated_by_id],
+                post_update=True,
+            )
+
+            __mapper_args__ = {
+                "version_id_col": version_id,
+            }
+
+    def test_bumped_version_id(self):
+        User, Parent = self.classes("User", "Parent")
+
+        session = fixture_session()
+        u1 = User(id=1)
+        u2 = User(id=2)
+        p1 = Parent(id=1, updated_by=u1)
+        session.add(u1)
+        session.add(u2)
+        session.add(p1)
+
+        u2id = u2.id
+        session.commit()
+        session.close()
+
+        p1 = session.get(Parent, 1)
+        p1.updated_by
+        p1.version_id = p1.version_id
+        p1.updated_by_id = u2id
+        assert "version_id" in inspect(p1).committed_state
+
+        with self.sql_execution_asserter(testing.db) as asserter:
+            session.commit()
+
+        asserter.assert_(
+            CompiledSQL(
+                "UPDATE parent SET version_id=:version_id, "
+                "updated_by_id=:updated_by_id WHERE parent.id = :parent_id "
+                "AND parent.version_id = :parent_version_id",
+                [
+                    {
+                        "version_id": 2,
+                        "updated_by_id": 2,
+                        "parent_id": 1,
+                        "parent_version_id": 1,
+                    }
+                ],
+            ),
+            CompiledSQL(
+                "UPDATE parent SET version_id=:version_id, "
+                "updated_by_id=:updated_by_id WHERE parent.id = :parent_id "
+                "AND parent.version_id = :parent_version_id",
+                [
+                    {
+                        "version_id": 3,
+                        "updated_by_id": 2,
+                        "parent_id": 1,
+                        "parent_version_id": 2,
+                    }
+                ],
+            ),
+        )