expunge pending orphans on flush that weren't caught by other means
Issue #4040
new
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:
Comments (4)
-
reporter -
reporter 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.
-
reporter 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
-
reporter - changed milestone to 1.3
- Log in to comment
lots of tests seem to assume this is not the behavior however so this is tentative.