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

diff --git a/doc/build/changelog/unreleased_20/10267.rst b/doc/build/changelog/unreleased_20/10267.rst
new file mode 100644 (file)
index 0000000..cfbf04f
--- /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 0593881e2de3fca7d1c21a3cfc1e50faf1b77c58..2fa281b8dd1fa62da89d7da058e169ece9a32b74 100644 (file)
--- 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 e4c102c0ad0b9ad205ed12a85f5c3f2c60d93193..88fb16a0049548932d7b73ede956cfba92bed473 100644 (file)
--- 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 ac122581a4fec820856ebe3694ff90ce7b974af8..b3fe217090486005fc8c4b1d861433df0e562aa8 100644 (file)
--- 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):