From f4d9c107554288318003437103a3ac3e2d0bbb51 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 5 Aug 2025 17:11:50 -0400 Subject: [PATCH] apply correct pre-fetch params to post updated rows Fixed issue where using the ``post_update`` feature would apply incorrect "pre-fetched" values to the ORM objects after a multi-row UPDATE process completed. These "pre-fetched" values would come from any column that had an :paramref:`.Column.onupdate` callable or a version id generator used by :paramref:`.orm.Mapper.version_id_generator`; for a version id generator that delivered random identifiers like timestamps or UUIDs, this incorrect data would lead to a DELETE statement against those same rows to fail in the next step. Fixes: #12748 Change-Id: Id12c7973f168604533762dfc01afbb9155b693a6 --- doc/build/changelog/unreleased_20/12748.rst | 13 ++++++ lib/sqlalchemy/orm/persistence.py | 10 ++++- test/orm/test_cycles.py | 42 ++++++++++++++++++++ test/orm/test_versioning.py | 44 +++++++++++++++++++++ 4 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/12748.rst diff --git a/doc/build/changelog/unreleased_20/12748.rst b/doc/build/changelog/unreleased_20/12748.rst new file mode 100644 index 0000000000..6891632953 --- /dev/null +++ b/doc/build/changelog/unreleased_20/12748.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, orm + :tickets: 12748 + + Fixed issue where using the ``post_update`` feature would apply incorrect + "pre-fetched" values to the ORM objects after a multi-row UPDATE process + completed. These "pre-fetched" values would come from any column that had + an :paramref:`.Column.onupdate` callable or a version id generator used by + :paramref:`.orm.Mapper.version_id_generator`; for a version id generator + that delivered random identifiers like timestamps or UUIDs, this incorrect + data would lead to a DELETE statement against those same rows to fail in + the next step. + diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index 1d6b4abf66..f720f90951 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -1379,7 +1379,13 @@ def _emit_post_update_statements( ) rows += c.rowcount - for state, state_dict, mapper_rec, connection, params in records: + for i, ( + state, + state_dict, + mapper_rec, + connection, + params, + ) in enumerate(records): _postfetch_post_update( mapper_rec, uowtransaction, @@ -1387,7 +1393,7 @@ def _emit_post_update_statements( state, state_dict, c, - c.context.compiled_parameters[0], + c.context.compiled_parameters[i], ) if check_rowcount: diff --git a/test/orm/test_cycles.py b/test/orm/test_cycles.py index fb37185f53..b4ddd26e77 100644 --- a/test/orm/test_cycles.py +++ b/test/orm/test_cycles.py @@ -15,7 +15,9 @@ from sqlalchemy import Integer from sqlalchemy import String from sqlalchemy import testing from sqlalchemy.orm import backref +from sqlalchemy.orm import mapped_column from sqlalchemy.orm import relationship +from sqlalchemy.orm import Session from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ @@ -1586,6 +1588,46 @@ class SelfReferentialPostUpdateTest3(fixtures.MappedTest): session.flush() +class PostUpdatePrefetchTest(fixtures.DeclarativeMappedTest): + """test #12748""" + + run_setup_classes = "each" + + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + count = 0 + + def _counter(): + nonlocal count + count += 1 + return count + + class Parent(Base): + __tablename__ = "parent" + id = mapped_column(Integer, primary_key=True) + + related = relationship("Related", post_update=True) + + class Related(Base): + __tablename__ = "related" + + id = mapped_column(Integer, primary_key=True) + parent_id = mapped_column(ForeignKey("parent.id")) + counter = mapped_column(Integer, onupdate=_counter) + + def test_update_counter(self, connection): + Parent, Related = self.classes("Parent", "Related") + + p1 = Parent(related=[Related(), Related(), Related()]) + with Session(connection, expire_on_commit=False) as sess: + sess.add(p1) + sess.commit() + + eq_([rel.counter for rel in p1.related], [1, 2, 3]) + + class PostUpdateBatchingTest(fixtures.MappedTest): """test that lots of post update cols batch together into a single UPDATE.""" diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index 46821fe055..06fb1b2a5f 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -15,8 +15,10 @@ from sqlalchemy import String from sqlalchemy import testing from sqlalchemy import TypeDecorator from sqlalchemy import util +from sqlalchemy import Uuid from sqlalchemy.orm import configure_mappers from sqlalchemy.orm import exc as orm_exc +from sqlalchemy.orm import mapped_column from sqlalchemy.orm import relationship from sqlalchemy.orm import Session from sqlalchemy.testing import assert_raises @@ -757,6 +759,48 @@ class VersionOnPostUpdateTest(fixtures.MappedTest): ) +class PostUpdatePrefetchTest(fixtures.DeclarativeMappedTest): + """test #12748""" + + run_setup_classes = "each" + + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class Parent(Base): + __tablename__ = "parent" + id = mapped_column(Integer, primary_key=True) + + related = relationship( + "Related", post_update=True, cascade="all, delete-orphan" + ) + + class Related(Base): + __tablename__ = "related" + + id = mapped_column(Integer, primary_key=True) + parent_id = mapped_column(ForeignKey("parent.id")) + version = mapped_column(Uuid) + + __mapper_args__ = { + "version_id_col": version, + "version_id_generator": lambda v: uuid.uuid4(), + } + + def test_random_versionids(self, connection): + Parent, Related = self.classes("Parent", "Related") + + p1 = Parent(related=[Related(), Related(), Related()]) + with Session(connection, expire_on_commit=False) as sess: + sess.add(p1) + sess.commit() + + with Session(connection, expire_on_commit=False) as sess: + sess.delete(p1) + sess.commit() + + class NoBumpOnRelationshipTest(fixtures.MappedTest): __backend__ = True -- 2.47.2