expunge pending orphans on flush that weren't caught by other means

[Mike Bayer]

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

Base = declarative_base()


class A1(Base):
    __tablename__ = 'a1'
    id = Column(Integer, primary_key=True)
    bs = relationship("B", cascade="all, delete-orphan")


class A2(Base):
    __tablename__ = 'a2'
    id = Column(Integer, primary_key=True)
    bs = relationship("B", cascade="all, delete-orphan")


class B(Base):
    __tablename__ = 'b'
    id = Column(Integer, primary_key=True)
    a1_id = Column(ForeignKey('a1.id'))
    a2_id = Column(ForeignKey('a2.id'))

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

s = Session(e)

b_orphan1, b_orphan2 = B(), B()

a1a, a1b = A1(), A1()

a2a = A2()

a2a.bs.append(b_orphan1)
a2a.bs.append(b_orphan2)

s.add(a2a)
s.add(a1a)

# add it here, it works
# s.add(a1b)

a1a.bs.append(b_orphan1)
a1b.bs.append(b_orphan2)

a1a.bs.remove(b_orphan1)
a1b.bs.remove(b_orphan2)

# down here, fails
s.add(a1b)

s.commit()

assert len(s.query(B).all()) == 0

fix is to check for pending orphans once more at the same time we check for persistent orphans:

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 7c313e6..bade700 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -2271,11 +2271,13 @@ class Session(_SessionClassMethods):
             proc = new.union(dirty).difference(deleted)

         for state in proc:
-            is_orphan = (
-                _state_mapper(state)._is_orphan(state) and state.has_identity)
-            _reg = flush_context.register_object(state, isdelete=is_orphan)
-            assert _reg, "Failed to add object to the flush context!"
-            processed.add(state)
+            is_orphan = _state_mapper(state)._is_orphan(state)
+            if is_orphan and not state.has_identity:
+                self._expunge_states([state])
+            else:
+                _reg = flush_context.register_object(state, isdelete=is_orphan)
+                assert _reg, "Failed to add object to the flush context!"
+                processed.add(state)

         # put all remaining deletes into the flush context.
         if objset:

[Mike Bayer]

lots of tests seem to assume this is not the behavior however so this is tentative.

[Mike Bayer]

so yes the above fails w the behavior that if you say, session.add(some_orphan), that should try to do the INSERT and fail. so the check does need to somehow link to the removal from the collection.

[Mike Bayer]

OK, here is a much more complex patch that unfortunately adds an ugly flag, and makes me want to push this to 1.3 as catching all the cases for this flag or not would need lots of new tests:

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 7c313e6..752f182 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1688,6 +1688,7 @@ class Session(_SessionClassMethods):
                     state.key = instance_key

                 self.identity_map.replace(state)
+                state._orphaned_outside_of_session = False

         statelib.InstanceState._commit_all_states(
             ((state, state.dict) for state in states),
@@ -1762,6 +1763,7 @@ class Session(_SessionClassMethods):
             self.add(instance, _warn=False)

     def _save_or_update_state(self, state):
+        state._orphaned_outside_of_session = False
         self._save_or_update_impl(state)

         mapper = _state_mapper(state)
@@ -2271,11 +2273,17 @@ class Session(_SessionClassMethods):
             proc = new.union(dirty).difference(deleted)

         for state in proc:
-            is_orphan = (
-                _state_mapper(state)._is_orphan(state) and state.has_identity)
-            _reg = flush_context.register_object(state, isdelete=is_orphan)
-            assert _reg, "Failed to add object to the flush context!"
-            processed.add(state)
+            is_orphan = _state_mapper(state)._is_orphan(state)
+
+            is_persistent_orphan = is_orphan and state.has_identity
+
+            if is_orphan and not is_persistent_orphan and state._orphaned_outside_of_session:
+                self._expunge_states([state])
+            else:
+                _reg = flush_context.register_object(
+                    state, isdelete=is_persistent_orphan)
+                assert _reg, "Failed to add object to the flush context!"
+                processed.add(state)

         # put all remaining deletes into the flush context.
         if objset:
diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index 1781a41..2e53fe9 100644
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -61,6 +61,7 @@ class InstanceState(interfaces.InspectionAttr):
     expired = False
     _deleted = False
     _load_pending = False
+    _orphaned_outside_of_session = False
     is_instance = True

     callables = ()
diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index ee3e204..a3bd536 100644
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -52,22 +52,24 @@ def track_cascade_events(descriptor, prop):
             return

         sess = state.session
-        if sess:

-            prop = state.manager.mapper._props[key]
+        prop = state.manager.mapper._props[key]

-            if sess._warn_on_events:
-                sess._flush_warning(
-                    "collection remove"
-                    if prop.uselist
-                    else "related attribute delete")
+        if sess and sess._warn_on_events:
+            sess._flush_warning(
+                "collection remove"
+                if prop.uselist
+                else "related attribute delete")

-            # expunge pending orphans
-            item_state = attributes.instance_state(item)
-            if prop._cascade.delete_orphan and \
-                item_state in sess._new and \
-                    prop.mapper._is_orphan(item_state):
+        # expunge pending orphans
+        item_state = attributes.instance_state(item)
+
+        if prop._cascade.delete_orphan and \
+                prop.mapper._is_orphan(item_state):
+            if sess and item_state in sess._new:
                 sess.expunge(item)
+            else:
+                item_state._orphaned_outside_of_session = True

     def set_(state, newvalue, oldvalue, initiator):
         # process "save_update" cascade rules for when an instance