double check conflicting "row switch" instance for previously deleted

test case:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()


class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)

e = create_engine("sqlite://", echo=True)
Base.metadata.drop_all(e)
Base.metadata.create_all(e)

s = Session(e, autocommit=True)

try:
    with s.begin():
        a1 = A(id=1)
        s.add(a1)

        s.flush()

        del a1

        a1 = s.query(A).first()

        # exception is raised
        raise Exception("something broke")
except Exception:
    with s.begin():
        a2 = A(id=1)
        s.add(a2)

raises:

#!

 File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/persistence.py", line 303, in _organize_states_for_save
    state_str(existing)))
sqlalchemy.orm.exc.FlushError: New instance <A at 0x7f2d0fe5bf90> with identity key (<class '__main__.A'>, (1,)) conflicts with persistent instance <A at 0x7f2d0f8ca050>

because A was loaded again and isn't in the "rollback" buffer.

if we look for expired objects and test them when we look for row switch we avoid this issue:

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 5d69f51..79ec669 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -293,22 +293,24 @@ def _organize_states_for_save(base_mapper, states, uowtransaction):
             instance = \
                 uowtransaction.session.identity_map[instance_key]
             existing = attributes.instance_state(instance)
-            if not uowtransaction.is_deleted(existing):
-                raise orm_exc.FlushError(
-                    "New instance %s with identity key %s conflicts "
-                    "with persistent instance %s" %
-                    (state_str(state), instance_key,
-                     state_str(existing)))
-
-            base_mapper._log_debug(
-                "detected row switch for identity %s.  "
-                "will update %s, remove %s from "
-                "transaction", instance_key,
-                state_str(state), state_str(existing))
-
-            # remove the "delete" flag from the existing element
-            uowtransaction.remove_state_actions(existing)
-            row_switch = existing
+
+            if not uowtransaction.was_already_deleted(existing):
+                if not uowtransaction.is_deleted(existing):
+                    raise orm_exc.FlushError(
+                        "New instance %s with identity key %s conflicts "
+                        "with persistent instance %s" %
+                        (state_str(state), instance_key,
+                         state_str(existing)))
+
+                base_mapper._log_debug(
+                    "detected row switch for identity %s.  "
+                    "will update %s, remove %s from "
+                    "transaction", instance_key,
+                    state_str(state), state_str(existing))
+
+                # remove the "delete" flag from the existing element
+                uowtransaction.remove_state_actions(existing)
+                row_switch = existing

         if (has_identity or row_switch) and mapper.version_id_col is not None:
             update_version_id = mapper._get_committed_state_attr_by_column(
diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index 8b4ae64..95a5401 100644
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -16,6 +16,7 @@ organizes them in order of dependency, and executes.
 from .. import util, event
 from ..util import topological
 from . import attributes, persistence, util as orm_util
+from . import exc as orm_exc
 import itertools


@@ -155,6 +156,19 @@ class UOWTransaction(object):
     def has_work(self):
         return bool(self.states)

+    def was_already_deleted(self, state):
+        """return true if the given state is expired and was deleted
+        previously.
+        """
+        if state.expired:
+            try:
+                # MARKMARK
+                state._load_expired(state, attributes.PASSIVE_OFF)
+            except orm_exc.ObjectDeletedError:
+                self.session._remove_newly_deleted([state])
+                return True
+        return False
+
     def is_deleted(self, state):
         """return true if the given state is marked as deleted
         within this uowtransaction."""

Comments (3)