session._update_impl adds the item to identity map even if error thrown

Issue #3301 resolved
Mike Bayer repo owner created an issue
from sqlalchemy import Column, String, Integer, ForeignKey
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Session, session

Base = declarative_base()

class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String)

s1 = Session()
s2 = Session()
u1 = User(id=1, name='u1')
session.make_transient_to_detached(u1)  # shorthand for actually persisting it
s1.add(u1)

try:
    s2.add(u1)
except:
    assert not s2.identity_map.keys()

patch:

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 0e272dc..2b26831 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1795,7 +1795,7 @@ class Session(_SessionClassMethods):
                 "function to send this object back to the transient state." %
                 state_str(state)
             )
-        self._before_attach(state)
+        self._before_attach(state, check_identity_map=False)
         self._deleted.pop(state, None)
         if discard_existing:
             self.identity_map.replace(state)
@@ -1875,13 +1875,12 @@ class Session(_SessionClassMethods):
         self._attach(state, include_before=True)
         state._load_pending = True

-    def _before_attach(self, state):
+    def _before_attach(self, state, check_identity_map=True):
         if state.session_id != self.hash_key and \
                 self.dispatch.before_attach:
             self.dispatch.before_attach(self, state.obj())

-    def _attach(self, state, include_before=False):
-        if state.key and \
+        if check_identity_map and state.key and \
             state.key in self.identity_map and \
                 not self.identity_map.contains_state(state):
             raise sa_exc.InvalidRequestError(
@@ -1897,10 +1896,11 @@ class Session(_SessionClassMethods):
                 "(this is '%s')" % (state_str(state),
                                     state.session_id, self.hash_key))

+    def _attach(self, state, include_before=False):
+
         if state.session_id != self.hash_key:
-            if include_before and \
-                    self.dispatch.before_attach:
-                self.dispatch.before_attach(self, state.obj())
+            if include_before:
+                self._before_attach(state)
             state.session_id = self.hash_key
             if state.modified and state._strong_obj is None:
                 state._strong_obj = state.obj()

Comments (1)

  1. Mike Bayer reporter
    • Fixed bug where the session attachment error "object is already attached to session X" would fail to prevent the object from also being attached to the new session, in the case that execution continued after the error raise occurred. fixes #3301

    → <<cset 66fa5b50a53e>>

  2. Log in to comment