From: Mike Bayer Date: Fri, 30 May 2014 05:39:45 +0000 (-0400) Subject: - Related to :ticket:`3060`, an adjustment has been made to the unit X-Git-Tag: rel_0_9_5~35 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5992c6096a67497aeb705939d1059bbfa410c87e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Related to :ticket:`3060`, an adjustment has been made to the unit of work such that loading for related many-to-one objects is slightly more aggressive, in the case of a graph of self-referential objects that are to be deleted; the load of related objects is to help determine the correct order for deletion if passive_deletes is not set. - revert the changes to test_delete_unloaded_m2o, these deletes do in fact need to occur in the order of the two child objects first. merged from master --- diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index 512dce0915..98a11a00d3 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -29,6 +29,17 @@ of implicitly assuming None isn't really a "change" for a previously un-set attribute. See also :ticket:`3061`. + .. change:: + :tags: bug, orm + :versions: 1.0.0 + + Related to :ticket:`3060`, an adjustment has been made to the unit + of work such that loading for related many-to-one objects is slightly + more aggressive, in the case of a graph of self-referential objects + that are to be deleted; the load of related objects is to help + determine the correct order for deletion if passive_deletes is + not set. + .. change:: :tags: bug, orm :tickets: 3057 diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 09d6e988d4..bcfd4016d8 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -532,7 +532,7 @@ class AttributeImpl(object): def get_history(self, state, dict_, passive=PASSIVE_OFF): raise NotImplementedError() - def get_all_pending(self, state, dict_): + def get_all_pending(self, state, dict_, passive=PASSIVE_NO_INITIALIZE): """Return a list of tuples of (state, obj) for all objects in this attribute's current state + history. @@ -737,23 +737,31 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): else: return History.from_object_attribute(self, state, current) - def get_all_pending(self, state, dict_): + def get_all_pending(self, state, dict_, passive=PASSIVE_NO_INITIALIZE): if self.key in dict_: current = dict_[self.key] - if current is not None: - ret = [(instance_state(current), current)] - else: - ret = [(None, None)] + elif passive & CALLABLES_OK: + current = self.get(state, dict_, passive=passive) + else: + return [] + + # can't use __hash__(), can't use __eq__() here + if current is not None and \ + current is not PASSIVE_NO_RESULT and \ + current is not NEVER_SET: + ret = [(instance_state(current), current)] + else: + ret = [(None, None)] - if self.key in state.committed_state: - original = state.committed_state[self.key] - if original not in (NEVER_SET, PASSIVE_NO_RESULT, None) and \ + if self.key in state.committed_state: + original = state.committed_state[self.key] + if original is not None and \ + original is not PASSIVE_NO_RESULT and \ + original is not NEVER_SET and \ original is not current: - ret.append((instance_state(original), original)) - return ret - else: - return [] + ret.append((instance_state(original), original)) + return ret def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF, check_old=None, pop=False): @@ -859,7 +867,9 @@ class CollectionAttributeImpl(AttributeImpl): else: return History.from_collection(self, state, current) - def get_all_pending(self, state, dict_): + def get_all_pending(self, state, dict_, passive=PASSIVE_NO_INITIALIZE): + # NOTE: passive is ignored here at the moment + if self.key not in dict_: return [] @@ -1087,7 +1097,9 @@ def backref_listeners(attribute, key, uselist): def emit_backref_from_scalar_set_event(state, child, oldchild, initiator): if oldchild is child: return child - if oldchild is not None and oldchild not in (PASSIVE_NO_RESULT, NEVER_SET): + if oldchild is not None and \ + oldchild is not PASSIVE_NO_RESULT and \ + oldchild is not NEVER_SET: # With lazy=None, there's no guarantee that the full collection is # present when updating via a backref. old_state, old_dict = instance_state(oldchild),\ diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 0d5a4f909e..bfe7aa0d25 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -154,12 +154,16 @@ class DependencyProcessor(object): parent_in_cycles = True # now create actions /dependencies for each state. + for state in states: # detect if there's anything changed or loaded - # by a preprocessor on this state/attribute. if not, - # we should be able to skip it entirely. + # by a preprocessor on this state/attribute. In the + # case of deletes we may try to load missing items here as well. sum_ = state.manager[self.key].impl.get_all_pending( - state, state.dict) + state, state.dict, + self._passive_delete_flag + if isdelete + else attributes.PASSIVE_NO_INITIALIZE) if not sum_: continue diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 2964705a2e..bb6a961b12 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -87,8 +87,9 @@ def track_cascade_events(descriptor, prop): sess._save_or_update_state(newvalue_state) if oldvalue is not None and \ + oldvalue is not attributes.NEVER_SET and \ oldvalue is not attributes.PASSIVE_NO_RESULT and \ - prop._cascade.delete_orphan: + prop._cascade.delete_orphan: # possible to reach here with attributes.NEVER_SET ? oldvalue_state = attributes.instance_state(oldvalue) diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 7025e087c5..00cc044bfa 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -1008,23 +1008,16 @@ class SingleCycleTest(UOWTest): "WHERE nodes.id = :param_1", lambda ctx: {'param_1': c2id} ), - Or( - AllOf( - CompiledSQL( - "DELETE FROM nodes WHERE nodes.id = :id", - lambda ctx: [{'id': c1id}, {'id': c2id}] - ), - CompiledSQL( - "DELETE FROM nodes WHERE nodes.id = :id", - lambda ctx: {'id': pid} - ), + AllOf( + CompiledSQL( + "DELETE FROM nodes WHERE nodes.id = :id", + lambda ctx: [{'id': c1id}, {'id': c2id}] ), CompiledSQL( "DELETE FROM nodes WHERE nodes.id = :id", - lambda ctx: [{'id': c1id}, {'id': c2id}, {'id': pid}] + lambda ctx: {'id': pid} ), - - ) + ), ), )