From ad480110ceebf47f99a28acb2a217e57eb745bbf Mon Sep 17 00:00:00 2001 From: Anubhav Banerjee Date: Thu, 9 Apr 2026 09:42:05 +0530 Subject: [PATCH 1/3] fix(#153): Ghost objects removedfrom the in-memory python list of the parent When we were reverting a relationship where a child object was marked for deletion, it was not being removed from the Python's in-memory data-structure (collection). This left 'ghost' objects in the collection. If an autoflush occurred, SQLAlchemy attempted to lazy-load deferred columns for these ghost objects, causing a KeyError: Deferred loader failed crash, which was especially prevalent if we had a polymorphic sub-class. This fix ensures the items are actively removed from the collection list in memory using collection.remove(value), keeping the Python state perfectly synced with the database session state. --- sqlalchemy_history/reverter.py | 5 +- tests/revert/test_polymorphic_relationship.py | 67 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 tests/revert/test_polymorphic_relationship.py diff --git a/sqlalchemy_history/reverter.py b/sqlalchemy_history/reverter.py index dca6dd1..d529caf 100644 --- a/sqlalchemy_history/reverter.py +++ b/sqlalchemy_history/reverter.py @@ -67,10 +67,11 @@ def revert_relationship(self, prop): value = self.revert_child(child_obj, prop) if value: values.append(value) - - for value in getattr(self.version_parent, prop.key, []): + collection = getattr(self.version_parent, prop.key, []) + for value in list(collection): if value not in values: self.session.delete(value) + collection.remove(value) else: self.revert_child(getattr(self.obj, prop.key), prop) diff --git a/tests/revert/test_polymorphic_relationship.py b/tests/revert/test_polymorphic_relationship.py new file mode 100644 index 0000000..21947d0 --- /dev/null +++ b/tests/revert/test_polymorphic_relationship.py @@ -0,0 +1,67 @@ +import pytest +import sqlalchemy as sa +from tests import TestCase + + +class TestRevertPolymorphicRelationship(TestCase): + def create_models(self): + class Car(self.Model): + __tablename__ = "car" + __versioned__ = {} + + id = sa.Column( + sa.Integer, sa.Sequence(f"{__tablename__}_seq", start=1), autoincrement=True, primary_key=True + ) + parts = sa.orm.relationship( + "Part", back_populates="car", cascade="all, delete-orphan", lazy="selectin" + ) + + class Part(self.Model): + __tablename__ = "part" + __versioned__ = {} + + id = sa.Column( + sa.Integer, sa.Sequence(f"{__tablename__}_seq", start=1), autoincrement=True, primary_key=True + ) + car_id = sa.Column(sa.Integer, sa.ForeignKey(Car.id)) + car = sa.orm.relationship(Car, back_populates="parts") + + type = sa.Column(sa.String(50)) + + __mapper_args__ = { + "polymorphic_identity": "part", + "polymorphic_on": type, + } + + class Tire(Part): + __tablename__ = "tire" + __versioned__ = {} + + id = sa.Column(sa.Integer, sa.ForeignKey(Part.id), primary_key=True) + radius = sa.Column(sa.Integer) + width = sa.Column(sa.Integer) + + __mapper_args__ = { + "polymorphic_identity": "tire", + } + + self.Car = Car + self.Part = Part + self.Tire = Tire + + def test_revert_polymorphic_relationship(self): + car = self.Car() + self.session.add(car) + self.session.commit() + + tire = self.Tire(radius=15, width=200) + car.parts.append(tire) + self.session.commit() + + initial_version = car.versions.all()[0] + reverted_car = initial_version.revert(relations=["parts"]) + + assert len(reverted_car.parts) == 0 + + with pytest.raises(IndexError): + print(reverted_car.parts[0]) From 21508102d70f9fee27333b4bc471c5c3e8bdc050 Mon Sep 17 00:00:00 2001 From: Anubhav Banerjee Date: Thu, 9 Apr 2026 16:58:05 +0530 Subject: [PATCH 2/3] Refactored the assign_attributes method to prevent the plugin from improperly triggering lazy-loads on deleted rows. --- sqlalchemy_history/unit_of_work.py | 12 ++++++++---- tests/revert/test_polymorphic_relationship.py | 3 +-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/sqlalchemy_history/unit_of_work.py b/sqlalchemy_history/unit_of_work.py index 9f09fbe..806e479 100644 --- a/sqlalchemy_history/unit_of_work.py +++ b/sqlalchemy_history/unit_of_work.py @@ -5,7 +5,7 @@ import sqlalchemy as sa from sqlalchemy_utils import get_primary_keys, identity -from sqlalchemy_history.operation import Operations +from sqlalchemy_history.operation import Operation, Operations from sqlalchemy_history.schema import update_end_tx_column from sqlalchemy_history.utils import ( end_tx_column_name, @@ -301,9 +301,13 @@ def assign_attributes(self, parent_obj, version_obj): :param version_obj: Version object to assign the attribute values to """ + state = sa.inspect(parent_obj) for prop in versioned_column_properties(parent_obj): - try: - value = getattr(parent_obj, prop.key) - except sa.orm.exc.ObjectDeletedError: + if version_obj.operation_type == Operation.DELETE and prop.key in state.unloaded: value = None + else: + try: + value = getattr(parent_obj, prop.key) + except sa.orm.exc.ObjectDeletedError: + value = None setattr(version_obj, prop.key, value) diff --git a/tests/revert/test_polymorphic_relationship.py b/tests/revert/test_polymorphic_relationship.py index 21947d0..80f634d 100644 --- a/tests/revert/test_polymorphic_relationship.py +++ b/tests/revert/test_polymorphic_relationship.py @@ -63,5 +63,4 @@ def test_revert_polymorphic_relationship(self): assert len(reverted_car.parts) == 0 - with pytest.raises(IndexError): - print(reverted_car.parts[0]) + self.session.flush() From 4a9d6701f9509ad23e8d8b3971d36318b451968f Mon Sep 17 00:00:00 2001 From: Anubhav Banerjee Date: Thu, 9 Apr 2026 17:28:05 +0530 Subject: [PATCH 3/3] Minor Syntax Fix: Removed unused import of Pytest --- tests/revert/test_polymorphic_relationship.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/revert/test_polymorphic_relationship.py b/tests/revert/test_polymorphic_relationship.py index 80f634d..785cf95 100644 --- a/tests/revert/test_polymorphic_relationship.py +++ b/tests/revert/test_polymorphic_relationship.py @@ -1,4 +1,3 @@ -import pytest import sqlalchemy as sa from tests import TestCase