diff options
| -rw-r--r-- | CHANGES | 3 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/attributes.py | 29 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/dependency.py | 122 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/dynamic.py | 2 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 20 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/sync.py | 8 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/unitofwork.py | 20 | ||||
| -rw-r--r-- | test/orm/unitofwork.py | 31 |
8 files changed, 140 insertions, 95 deletions
@@ -32,6 +32,9 @@ CHANGES - polymorphic_union() function respects the "key" of each Column if they differ from the column's name. + - Repaired support for "passive-deletes" on a many-to-one + relation() with "delete" cascade. [ticket:1183] + - Added more granularity to internal attribute access, such that cascade and flush operations will not initialize unloaded attributes and collections, leaving them intact for a diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index bc8f701e7..36a532faa 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -511,7 +511,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): else: current = self.get(state, passive=passive) if current is PASSIVE_NORESULT: - return (None, None, None) + return HISTORY_BLANK else: return History.from_attribute(self, state, current) @@ -590,7 +590,7 @@ class CollectionAttributeImpl(AttributeImpl): def get_history(self, state, passive=PASSIVE_OFF): current = self.get(state, passive=passive) if current is PASSIVE_NORESULT: - return (None, None, None) + return HISTORY_BLANK else: return History.from_attribute(self, state, current) @@ -1387,7 +1387,29 @@ class History(tuple): def __new__(cls, added, unchanged, deleted): return tuple.__new__(cls, (added, unchanged, deleted)) - + + def __nonzero__(self): + return self != HISTORY_BLANK + + def sum(self): + return self.added + self.unchanged + self.deleted + + def non_deleted(self): + return self.added + self.unchanged + + def non_added(self): + return self.unchanged + self.deleted + + def has_changes(self): + return bool(self.added or self.deleted) + + def as_state(self): + return History( + [c is not None and instance_state(c) or None for c in self.added], + [c is not None and instance_state(c) or None for c in self.unchanged], + [c is not None and instance_state(c) or None for c in self.deleted], + ) + @classmethod def from_attribute(cls, attribute, state, current): original = state.committed_state.get(attribute.key, NEVER_SET) @@ -1424,6 +1446,7 @@ class History(tuple): deleted = () return cls([current], (), deleted) +HISTORY_BLANK = History(None, None, None) class PendingCollection(object): """A writable placeholder for an unloaded collection. diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index b33213a80..2d0b7f1c6 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -169,32 +169,32 @@ class OneToManyDP(DependencyProcessor): # is on. if self.post_update or not self.passive_deletes == 'all': for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) - if unchanged or deleted: - for child in deleted: + history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) + if history: + for child in history.deleted: if child is not None and self.hasparent(child) is False: self._synchronize(state, child, None, True, uowcommit) self._conditional_post_update(child, uowcommit, [state]) if self.post_update or not self.cascade.delete: - for child in unchanged: + for child in history.unchanged: if child is not None: self._synchronize(state, child, None, True, uowcommit) self._conditional_post_update(child, uowcommit, [state]) else: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True) - if added or deleted: - for child in added: + history = uowcommit.get_attribute_history(state, self.key, passive=True) + if history: + for child in history.added: self._synchronize(state, child, None, False, uowcommit) if child is not None: self._conditional_post_update(child, uowcommit, [state]) - for child in deleted: + + for child in history.deleted: if not self.cascade.delete_orphan and not self.hasparent(child): self._synchronize(state, child, None, True, uowcommit) - if self._pks_changed(uowcommit, state): - if unchanged: - for child in unchanged: + if self._pks_changed(uowcommit, state): + for child in history.unchanged: self._synchronize(state, child, None, False, uowcommit) def preprocess_dependencies(self, task, deplist, uowcommit, delete = False): @@ -206,26 +206,26 @@ class OneToManyDP(DependencyProcessor): if not self.post_update: should_null_fks = not self.cascade.delete and not self.passive_deletes == 'all' for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) - if unchanged or deleted: - for child in deleted: + history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) + if history: + for child in history.deleted: if child is not None and self.hasparent(child) is False: if self.cascade.delete_orphan: uowcommit.register_object(child, isdelete=True) else: uowcommit.register_object(child) if should_null_fks: - for child in unchanged: + for child in history.unchanged: if child is not None: uowcommit.register_object(child) else: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True) - if added or deleted: - for child in added: + history = uowcommit.get_attribute_history(state, self.key, passive=True) + if history: + for child in history.added: if child is not None: uowcommit.register_object(child) - for child in deleted: + for child in history.deleted: if not self.cascade.delete_orphan: uowcommit.register_object(child, isdelete=False) elif self.hasparent(child) is False: @@ -235,11 +235,10 @@ class OneToManyDP(DependencyProcessor): attributes.instance_state(c), isdelete=True) if not self.passive_updates and self._pks_changed(uowcommit, state): - if not unchanged: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=False) - if unchanged: - for child in unchanged: - uowcommit.register_object(child) + if not history: + history = uowcommit.get_attribute_history(state, self.key, passive=False) + for child in history.unchanged: + uowcommit.register_object(child) def _synchronize(self, state, child, associationrow, clearkeys, uowcommit): source = state @@ -314,7 +313,7 @@ class ManyToOneDP(DependencyProcessor): uowcommit.register_processor(self.mapper, self, self.parent) - def process_dependencies(self, task, deplist, uowcommit, delete = False): + def process_dependencies(self, task, deplist, uowcommit, delete=False): #print self.mapper.mapped_table.name + " " + self.key + " " + repr(len(deplist)) + " process_dep isdelete " + repr(delete) + " direction " + repr(self.direction) if delete: if self.post_update and not self.cascade.delete_orphan and not self.passive_deletes == 'all': @@ -322,43 +321,44 @@ class ManyToOneDP(DependencyProcessor): # before we can DELETE the row for state in deplist: self._synchronize(state, None, None, True, uowcommit) - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) - if added or unchanged or deleted: - self._conditional_post_update(state, uowcommit, deleted + unchanged + added) + history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) + if history: + self._conditional_post_update(state, uowcommit, history.sum()) else: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True) - if added or deleted or unchanged: - for child in added: + history = uowcommit.get_attribute_history(state, self.key, passive=True) + if history: + for child in history.added: self._synchronize(state, child, None, False, uowcommit) - self._conditional_post_update(state, uowcommit, deleted + unchanged + added) + self._conditional_post_update(state, uowcommit, history.sum()) - def preprocess_dependencies(self, task, deplist, uowcommit, delete = False): + def preprocess_dependencies(self, task, deplist, uowcommit, delete=False): #print self.mapper.mapped_table.name + " " + self.key + " " + repr(len(deplist)) + " PRE process_dep isdelete " + repr(delete) + " direction " + repr(self.direction) if self.post_update: return if delete: if self.cascade.delete or self.cascade.delete_orphan: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) - if self.cascade.delete_orphan: - todelete = added + unchanged + deleted - else: - todelete = added + unchanged - for child in todelete: - if child is None: - continue - uowcommit.register_object(child, isdelete=True) - for c, m in self.mapper.cascade_iterator('delete', child): - uowcommit.register_object( - attributes.instance_state(c), isdelete=True) + history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) + if history: + if self.cascade.delete_orphan: + todelete = history.sum() + else: + todelete = history.non_deleted() + for child in todelete: + if child is None: + continue + uowcommit.register_object(child, isdelete=True) + for c, m in self.mapper.cascade_iterator('delete', child): + uowcommit.register_object( + attributes.instance_state(c), isdelete=True) else: for state in deplist: uowcommit.register_object(state) if self.cascade.delete_orphan: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) - if deleted: - for child in deleted: + history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) + if history: + for child in history.deleted: if self.hasparent(child) is False: uowcommit.register_object(child, isdelete=True) for c, m in self.mapper.cascade_iterator('delete', child): @@ -403,9 +403,9 @@ class ManyToManyDP(DependencyProcessor): if delete: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) - if deleted or unchanged: - for child in deleted + unchanged: + history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes) + if history: + for child in history.non_added(): if child is None or (reverse_dep and (reverse_dep, "manytomany", child, state) in uowcommit.attributes): continue associationrow = {} @@ -414,16 +414,16 @@ class ManyToManyDP(DependencyProcessor): uowcommit.attributes[(self, "manytomany", state, child)] = True else: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key) - if added or deleted: - for child in added: + history = uowcommit.get_attribute_history(state, self.key) + if history: + for child in history.added: if child is None or (reverse_dep and (reverse_dep, "manytomany", child, state) in uowcommit.attributes): continue associationrow = {} self._synchronize(state, child, associationrow, False, uowcommit) uowcommit.attributes[(self, "manytomany", state, child)] = True secondary_insert.append(associationrow) - for child in deleted: + for child in history.deleted: if child is None or (reverse_dep and (reverse_dep, "manytomany", child, state) in uowcommit.attributes): continue associationrow = {} @@ -432,10 +432,10 @@ class ManyToManyDP(DependencyProcessor): secondary_delete.append(associationrow) if not self.passive_updates and self._pks_changed(uowcommit, state): - if not unchanged: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=False) + if not history: + history = uowcommit.get_attribute_history(state, self.key, passive=False) - for child in unchanged: + for child in history.unchanged: associationrow = {} sync.update(state, self.parent, associationrow, "old_", self.prop.synchronize_pairs) sync.update(child, self.mapper, associationrow, "old_", self.prop.secondary_synchronize_pairs) @@ -465,9 +465,9 @@ class ManyToManyDP(DependencyProcessor): #print self.mapper.mapped_table.name + " " + self.key + " " + repr(len(deplist)) + " preprocess_dep isdelete " + repr(delete) + " direction " + repr(self.direction) if not delete: for state in deplist: - (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True) - if deleted: - for child in deleted: + history = uowcommit.get_attribute_history(state, self.key, passive=True) + if history: + for child in history.deleted: if self.cascade.delete_orphan and self.hasparent(child) is False: uowcommit.register_object(child, isdelete=True) for c, m in self.mapper.cascade_iterator('delete', child): diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index 8c77547da..eed50cd6d 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -100,7 +100,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl): def get_history(self, state, passive=False): c = self._get_collection_history(state, passive) - return (c.added_items, c.unchanged_items, c.deleted_items) + return attributes.History(c.added_items, c.unchanged_items, c.deleted_items) def _get_collection_history(self, state, passive=False): if self.key in state.committed_state: diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 279fcd60c..be1be549c 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1148,8 +1148,8 @@ class Mapper(object): params[col._label] = mapper._get_state_attr_by_column(state, col) params[col.key] = params[col._label] + 1 for prop in mapper._columntoproperty.values(): - (added, unchanged, deleted) = attributes.get_history(state, prop.key, passive=True) - if added: + history = attributes.get_history(state, prop.key, passive=True) + if history.added: hasdata = True elif mapper.polymorphic_on and mapper.polymorphic_on.shares_lineage(col): pass @@ -1160,18 +1160,18 @@ class Mapper(object): continue prop = mapper._columntoproperty[col] - (added, unchanged, deleted) = attributes.get_history(state, prop.key, passive=True) - if added: - if isinstance(added[0], sql.ClauseElement): - value_params[col] = added[0] + history = attributes.get_history(state, prop.key, passive=True) + if history.added: + if isinstance(history.added[0], sql.ClauseElement): + value_params[col] = history.added[0] else: - params[col.key] = prop.get_col_value(col, added[0]) + params[col.key] = prop.get_col_value(col, history.added[0]) if col in pks: - if deleted: - params[col._label] = deleted[0] + if history.deleted: + params[col._label] = history.deleted[0] else: # row switch logic can reach us here - params[col._label] = added[0] + params[col._label] = history.added[0] hasdata = True elif col in pks: params[col._label] = mapper._get_state_attr_by_column(state, col) diff --git a/lib/sqlalchemy/orm/sync.py b/lib/sqlalchemy/orm/sync.py index eca80df25..f338b331f 100644 --- a/lib/sqlalchemy/orm/sync.py +++ b/lib/sqlalchemy/orm/sync.py @@ -56,8 +56,8 @@ def source_changes(uowcommit, source, source_mapper, synchronize_pairs): prop = source_mapper._get_col_to_prop(l) except exc.UnmappedColumnError: _raise_col_to_prop(False, source_mapper, l, None, r) - (added, unchanged, deleted) = uowcommit.get_attribute_history(source, prop.key, passive=True) - if added and deleted: + history = uowcommit.get_attribute_history(source, prop.key, passive=True) + if history.has_changes(): return True else: return False @@ -68,8 +68,8 @@ def dest_changes(uowcommit, dest, dest_mapper, synchronize_pairs): prop = dest_mapper._get_col_to_prop(r) except exc.UnmappedColumnError: _raise_col_to_prop(True, None, l, dest_mapper, r) - (added, unchanged, deleted) = uowcommit.get_attribute_history(dest, prop.key, passive=True) - if added and deleted: + history = uowcommit.get_attribute_history(dest, prop.key, passive=True) + if history.has_changes(): return True else: return False diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 0992bfe99..778bf0949 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -119,24 +119,20 @@ class UOWTransaction(object): # prevents newly loaded objects from being dereferenced during the # flush process if hashkey in self.attributes: - (added, unchanged, deleted, cached_passive) = self.attributes[hashkey] + (history, cached_passive) = self.attributes[hashkey] # if the cached lookup was "passive" and now we want non-passive, do a non-passive # lookup and re-cache if cached_passive and not passive: - (added, unchanged, deleted) = attributes.get_history(state, key, passive=False) - self.attributes[hashkey] = (added, unchanged, deleted, passive) + history = attributes.get_history(state, key, passive=False) + self.attributes[hashkey] = (history, passive) else: - (added, unchanged, deleted) = attributes.get_history(state, key, passive=passive) - self.attributes[hashkey] = (added, unchanged, deleted, passive) + history = attributes.get_history(state, key, passive=passive) + self.attributes[hashkey] = (history, passive) - if added is None or not state.get_impl(key).uses_objects: - return (added, unchanged, deleted) + if not history or not state.get_impl(key).uses_objects: + return history else: - return ( - [c is not None and attributes.instance_state(c) or None for c in added], - [c is not None and attributes.instance_state(c) or None for c in unchanged], - [c is not None and attributes.instance_state(c) or None for c in deleted], - ) + return history.as_state() def register_object(self, state, isdelete=False, listonly=False, postupdate=False, post_update_cols=None): # if object is not in the overall session, do nothing diff --git a/test/orm/unitofwork.py b/test/orm/unitofwork.py index 05f4d88f3..975743fb0 100644 --- a/test/orm/unitofwork.py +++ b/test/orm/unitofwork.py @@ -694,15 +694,12 @@ class PassiveDeletesTest(_base.MappedTest): pass @testing.resolve_artifact_names - def setup_mappers(self): + def test_basic(self): mapper(MyOtherClass, myothertable) mapper(MyClass, mytable, properties={ 'children':relation(MyOtherClass, passive_deletes=True, cascade="all")}) - - @testing.resolve_artifact_names - def test_basic(self): session = create_session() mc = MyClass() mc.children.append(MyOtherClass()) @@ -721,7 +718,33 @@ class PassiveDeletesTest(_base.MappedTest): assert mytable.count().scalar() == 0 assert myothertable.count().scalar() == 0 + + @testing.resolve_artifact_names + def test_backwards_pd(self): + # the unusual scenario where a trigger or something might be deleting + # a many-to-one on deletion of the parent row + mapper(MyOtherClass, myothertable, properties={ + 'myclass':relation(MyClass, cascade="all, delete", passive_deletes=True) + }) + mapper(MyClass, mytable) + + session = create_session() + mc = MyClass() + mco = MyOtherClass() + mco.myclass = mc + session.add(mco) + session.flush() + assert mytable.count().scalar() == 1 + assert myothertable.count().scalar() == 1 + + session.expire(mco, ['myclass']) + session.delete(mco) + session.flush() + + assert mytable.count().scalar() == 1 + assert myothertable.count().scalar() == 0 + class ExtraPassiveDeletesTest(_base.MappedTest): __requires__ = ('foreign_keys',) |