From 6a59eecfa891db84033f5d0c88451b344e5b6f0c Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 1 Aug 2024 16:41:45 -0400 Subject: [PATCH] add check for pre-existing history records Fixed issue in history_meta example where the "version" column in the versioned table needs to default to the most recent version number in the history table on INSERT, to suit the use case of a table where rows are deleted, and can then be replaced by new rows that re-use the same primary key identity. This fix adds an additonal SELECT query per INSERT in the main table, which may be inefficient; for cases where primary keys are not re-used, the default function may be omitted. Patch courtesy Philipp H. v. Loewenfeld. Fixes: #10267 Change-Id: I6b0737a7e871763f95fd636c9ad98b80f3b5808e --- doc/build/changelog/unreleased_20/10267.rst | 13 ++++ examples/versioned_history/__init__.py | 6 +- examples/versioned_history/history_meta.py | 36 ++++++++- examples/versioned_history/test_versioning.py | 73 +++++++++++++++++++ 4 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/10267.rst diff --git a/doc/build/changelog/unreleased_20/10267.rst b/doc/build/changelog/unreleased_20/10267.rst new file mode 100644 index 0000000000..cfbf04f6db --- /dev/null +++ b/doc/build/changelog/unreleased_20/10267.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, examples + :tickets: 10267 + + Fixed issue in history_meta example where the "version" column in the + versioned table needs to default to the most recent version number in the + history table on INSERT, to suit the use case of a table where rows are + deleted, and can then be replaced by new rows that re-use the same primary + key identity. This fix adds an additonal SELECT query per INSERT in the + main table, which may be inefficient; for cases where primary keys are not + re-used, the default function may be omitted. Patch courtesy Philipp H. + v. Loewenfeld. + diff --git a/examples/versioned_history/__init__.py b/examples/versioned_history/__init__.py index 0593881e2d..2fa281b8dd 100644 --- a/examples/versioned_history/__init__.py +++ b/examples/versioned_history/__init__.py @@ -6,10 +6,10 @@ class which represents historical versions of the target object. Compare to the :ref:`examples_versioned_rows` examples which write updates as new rows in the same table, without using a separate history table. -Usage is illustrated via a unit test module ``test_versioning.py``, which can -be run like any other module, using ``unittest`` internally:: +Usage is illustrated via a unit test module ``test_versioning.py``, which is +run using SQLAlchemy's internal pytest plugin:: - python -m examples.versioned_history.test_versioning + pytest test/base/test_examples.py A fragment of example usage, using declarative:: diff --git a/examples/versioned_history/history_meta.py b/examples/versioned_history/history_meta.py index e4c102c0ad..88fb16a004 100644 --- a/examples/versioned_history/history_meta.py +++ b/examples/versioned_history/history_meta.py @@ -2,13 +2,16 @@ import datetime +from sqlalchemy import and_ from sqlalchemy import Column from sqlalchemy import DateTime from sqlalchemy import event from sqlalchemy import ForeignKeyConstraint +from sqlalchemy import func from sqlalchemy import inspect from sqlalchemy import Integer from sqlalchemy import PrimaryKeyConstraint +from sqlalchemy import select from sqlalchemy import util from sqlalchemy.orm import attributes from sqlalchemy.orm import object_mapper @@ -148,8 +151,39 @@ def _history_mapper(local_mapper): super_history_table.append_column(col) if not super_mapper: + + def default_version_from_history(context): + # Set default value of version column to the maximum of the + # version in history columns already present +1 + # Otherwise re-appearance of deleted rows would cause an error + # with the next update + current_parameters = context.get_current_parameters() + return context.connection.scalar( + select( + func.coalesce(func.max(history_table.c.version), 0) + 1 + ).where( + and_( + *[ + history_table.c[c.name] + == current_parameters.get(c.name, None) + for c in inspect( + local_mapper.local_table + ).primary_key + ] + ) + ) + ) + local_mapper.local_table.append_column( - Column("version", Integer, default=1, nullable=False), + Column( + "version", + Integer, + # if rows are not being deleted from the main table with + # subsequent re-use of primary key, this default can be + # "1" instead of running a query per INSERT + default=default_version_from_history, + nullable=False, + ), replace_existing=True, ) local_mapper.add_property( diff --git a/examples/versioned_history/test_versioning.py b/examples/versioned_history/test_versioning.py index ac122581a4..b3fe217090 100644 --- a/examples/versioned_history/test_versioning.py +++ b/examples/versioned_history/test_versioning.py @@ -881,6 +881,79 @@ class TestVersioning(AssertsCompiledSQL): sc2.name = "sc2 modified" sess.commit() + def test_external_id(self): + class ObjectExternal(Versioned, self.Base, ComparableEntity): + __tablename__ = "externalobjects" + + id1 = Column(String(3), primary_key=True) + id2 = Column(String(3), primary_key=True) + name = Column(String(50)) + + self.create_tables() + sess = self.session + sc = ObjectExternal(id1="aaa", id2="bbb", name="sc1") + sess.add(sc) + sess.commit() + + sc.name = "sc1modified" + sess.commit() + + assert sc.version == 2 + + ObjectExternalHistory = ObjectExternal.__history_mapper__.class_ + + eq_( + sess.query(ObjectExternalHistory).all(), + [ + ObjectExternalHistory( + version=1, id1="aaa", id2="bbb", name="sc1" + ), + ], + ) + + sess.delete(sc) + sess.commit() + + assert sess.query(ObjectExternal).count() == 0 + + eq_( + sess.query(ObjectExternalHistory).all(), + [ + ObjectExternalHistory( + version=1, id1="aaa", id2="bbb", name="sc1" + ), + ObjectExternalHistory( + version=2, id1="aaa", id2="bbb", name="sc1modified" + ), + ], + ) + + sc = ObjectExternal(id1="aaa", id2="bbb", name="sc1reappeared") + sess.add(sc) + sess.commit() + + assert sc.version == 3 + + sc.name = "sc1reappearedmodified" + sess.commit() + + assert sc.version == 4 + + eq_( + sess.query(ObjectExternalHistory).all(), + [ + ObjectExternalHistory( + version=1, id1="aaa", id2="bbb", name="sc1" + ), + ObjectExternalHistory( + version=2, id1="aaa", id2="bbb", name="sc1modified" + ), + ObjectExternalHistory( + version=3, id1="aaa", id2="bbb", name="sc1reappeared" + ), + ], + ) + class TestVersioningNewBase(TestVersioning): def make_base(self): -- 2.47.2