From: Mike Bayer Date: Thu, 8 Apr 2010 22:21:02 +0000 (-0400) Subject: starting to arrange things such that unneeded executors aren't getting X-Git-Tag: rel_0_6_0~36 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1f960f44c0c42c197caeb47ce075bac06c732def;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git starting to arrange things such that unneeded executors aren't getting pulled into the unit of work at all. this involves dancing around lists of states, seeing if child objects exist, not adding excessive callcounts while doing that, etc. --- diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index b631ea2c9f..dee0ec9c7f 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -1257,6 +1257,12 @@ class History(tuple): def __nonzero__(self): return self != HISTORY_BLANK + def empty(self): + return not bool( + (self.added or self.deleted) + or self.unchanged and self.unchanged != [None] + ) + def sum(self): return (self.added or []) +\ (self.unchanged or []) +\ diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 98840800f3..d94c6b43f5 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -70,6 +70,11 @@ class DependencyProcessor(object): unitofwork.GetDependentObjects(uow, self, False, True) unitofwork.GetDependentObjects(uow, self, True, True) + + + def _has_flush_activity(self, uow): + if self.post_update and self._check_reverse(uow): + return after_save = unitofwork.ProcessAll(uow, self, False, True) before_delete = unitofwork.ProcessAll(uow, self, True, True) @@ -99,6 +104,7 @@ class DependencyProcessor(object): child_deletes, after_save, before_delete) + def per_state_flush_actions(self, uow, states, isdelete): """establish actions and dependencies related to a flush. @@ -109,17 +115,26 @@ class DependencyProcessor(object): """ + # TODO: this check sucks. somehow get mapper to + # not even call this. + if ('has_flush_activity', self) not in uow.attributes: + return + + # TODO: why are we calling this ? shouldnt per_property + # have stopped us from getting keyhere ? if self.post_update and self._check_reverse(uow): # TODO: coverage here - return iter([]) + return # locate and disable the aggregate processors # for this dependency - before_delete = unitofwork.ProcessAll(uow, self, True, True) - before_delete.disabled = True - after_save = unitofwork.ProcessAll(uow, self, False, True) - after_save.disabled = True + if isdelete: + before_delete = unitofwork.ProcessAll(uow, self, True, True) + before_delete.disabled = True + else: + after_save = unitofwork.ProcessAll(uow, self, False, True) + after_save.disabled = True # check if the "child" side is part of the cycle @@ -222,15 +237,12 @@ class DependencyProcessor(object): after_save, before_delete, isdelete, childisdelete) - # ... but at the moment it - # does so we emit a null iterator - return iter([]) def presort_deletes(self, uowcommit, states): - pass + return False def presort_saves(self, uowcommit, states): - pass + return False def process_deletes(self, uowcommit, states): pass @@ -238,6 +250,20 @@ class DependencyProcessor(object): def process_saves(self, uowcommit, states): pass + def _prop_has_changes(self, uowcommit, states): + for s in states: + # TODO: add a high speed method + # to InstanceState which returns: attribute + # has a non-None value, or had one + history = uowcommit.get_attribute_history( + s, + self.key, + passive=True) + if history and not history.empty(): + return True + else: + return False + def _verify_canload(self, state): if state is not None and \ not self.mapper._canload(state, allow_subtypes=not self.enable_typechecks): @@ -266,11 +292,8 @@ class DependencyProcessor(object): """ for p in self.prop._reverse_property: - if not p.viewonly and p._dependency_processor and \ - (unitofwork.ProcessAll, - p._dependency_processor, False, True) in \ - uow.postsort_actions: - return True + if not p.viewonly and p._dependency_processor: + return p.key < self.key else: return False @@ -367,6 +390,7 @@ class OneToManyDP(DependencyProcessor): # foreign key to the parent set to NULL should_null_fks = not self.cascade.delete and \ not self.passive_deletes == 'all' + for state in states: history = uowcommit.get_attribute_history( state, @@ -383,7 +407,8 @@ class OneToManyDP(DependencyProcessor): for child in history.unchanged: if child is not None: uowcommit.register_object(child) - + + def presort_saves(self, uowcommit, states): for state in states: history = uowcommit.get_attribute_history( @@ -415,7 +440,7 @@ class OneToManyDP(DependencyProcessor): child, False, self.passive_updates) - + def process_deletes(self, uowcommit, states): # head object is being deleted, and we manage its list of # child objects the child objects have to have their foreign @@ -587,7 +612,7 @@ class ManyToOneDP(DependencyProcessor): 'delete', child): uowcommit.register_object( attributes.instance_state(c), isdelete=True) - + def presort_saves(self, uowcommit, states): for state in states: uowcommit.register_object(state) @@ -597,6 +622,7 @@ class ManyToOneDP(DependencyProcessor): self.key, passive=self.passive_deletes) if history: + ret = True for child in history.deleted: if self.hasparent(child) is False: uowcommit.register_object(child, isdelete=True) @@ -674,6 +700,10 @@ class DetectKeySwitch(DependencyProcessor): # so that mapper save_obj() gets a hold of changes unitofwork.GetDependentObjects(uow, self, False, False) else: + + ##### TODO ######## + # Get this out of here if self.parent.base_mapper isn't in the flush!!!! + # for passive updates, register objects in the process stage # so that we avoid ManyToOneDP's registering the object without # the listonly flag in its own preprocess stage (results in UPDATE) @@ -686,9 +716,11 @@ class DetectKeySwitch(DependencyProcessor): (parent_saves, after_save) ]) + def _has_flush_activity(self, uow): + pass + def per_state_flush_actions(self, uow, states, isdelete): - # TODO: coverage here - return iter([]) + pass def presort_deletes(self, uowcommit, states): assert False @@ -742,12 +774,18 @@ class ManyToManyDP(DependencyProcessor): unitofwork.GetDependentObjects(uow, self, False, True) else: DependencyProcessor.per_property_flush_actions(self, uow) + + def _has_flush_activity(self, uow): + if self._check_reverse(uow): + return + else: + DependencyProcessor._has_flush_activity(self, uow) def per_state_flush_actions(self, uow, states, isdelete): if self._check_reverse(uow): - return iter([]) + return else: - return DependencyProcessor.\ + DependencyProcessor.\ per_state_flush_actions(self, uow, states, isdelete) def per_property_dependencies(self, uow, parent_saves, @@ -798,9 +836,20 @@ class ManyToManyDP(DependencyProcessor): ]) def presort_deletes(self, uowcommit, states): - pass - + if not self.passive_deletes: + for state in states: + history = uowcommit.get_attribute_history( + state, + self.key, + passive=self.passive_deletes) + def presort_saves(self, uowcommit, states): + for state in states: + history = uowcommit.get_attribute_history( + state, + self.key, + False) + if not self.cascade.delete_orphan: return @@ -818,7 +867,7 @@ class ManyToManyDP(DependencyProcessor): child): uowcommit.register_object( attributes.instance_state(c), isdelete=True) - + def process_deletes(self, uowcommit, states): secondary_delete = [] secondary_insert = [] diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 511f4dfdfc..0ea0342c40 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1306,14 +1306,15 @@ class Mapper(object): mappers_to_states[state.manager.mapper].add(state) yield action + # TODO: can't we just loop through the frigging entries + # that are already in the uow instead of this goofy + # polymorphic BS ? for prop, mappers in self._property_iterator(set(mappers_to_states)): states_for_prop = [] for mapper in mappers: states_for_prop += list(mappers_to_states[mapper]) - - for rec in prop.per_state_flush_actions(uow, states_for_prop, isdelete): - yield rec - + + prop.per_state_flush_actions(uow, states_for_prop, isdelete) def _save_obj(self, states, uowtransaction, postupdate=False, post_update_cols=None, single=False): diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index b7601df144..bc715a03b0 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -1217,10 +1217,9 @@ class RelationshipProperty(StrategizedProperty): if not self.viewonly and self._dependency_processor: self._dependency_processor.per_property_flush_actions(uow) - def per_state_flush_actions(self, uow, state, isdelete): + def per_state_flush_actions(self, uow, states, isdelete): if not self.viewonly and self._dependency_processor: - for rec in self._dependency_processor.per_state_flush_actions(uow, state, isdelete): - yield rec + self._dependency_processor.per_state_flush_actions(uow, states, isdelete) def _create_joins(self, source_polymorphic=False, source_selectable=None, dest_polymorphic=False, diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 85ed790d9f..3d5bf8b946 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -208,10 +208,11 @@ class UOWTransaction(object): ] ).difference(cycles) - #sort = topological.sort(self.dependencies, postsort_actions) + sort = topological.sort(self.dependencies, postsort_actions) #print "--------------" #print self.dependencies - #print postsort_actions + print postsort_actions + print "COUNT OF POSTSORT ACTIONS", len(postsort_actions) # execute if cycles: @@ -318,6 +319,7 @@ class GetDependentObjects(PropertyRecMixin, PreSortRec): self.processed = set() def execute(self, uow): + states = list(self._elements(uow)) if states: if self.delete: @@ -325,6 +327,13 @@ class GetDependentObjects(PropertyRecMixin, PreSortRec): else: self.dependency_processor.presort_saves(uow, states) self.processed.update(states) + + if ('has_flush_activity', self.dependency_processor) not in uow.attributes: + # TODO: wont be calling self.fromparent here once detectkeyswitch works + if not self.fromparent or self.dependency_processor._prop_has_changes(uow, states): + self.dependency_processor._has_flush_activity(uow) + uow.attributes[('has_flush_activity', self.dependency_processor)] = True + return True else: return False @@ -338,6 +347,11 @@ class ProcessAll(PropertyRecMixin, PostSortRec): self.dependency_processor.process_saves(uow, states) def per_state_flush_actions(self, uow): + # we let the mappers call this, + # so that a ProcessAll which is between two mappers that + # are part of a cycle (but the ProcessAll itself is not + # in the cycle), also becomes a per-state processor, + # and a dependency between the two states as well return iter([]) class SaveUpdateAll(PostSortRec): diff --git a/lib/sqlalchemy/test/assertsql.py b/lib/sqlalchemy/test/assertsql.py index 81a6191a18..d67de23552 100644 --- a/lib/sqlalchemy/test/assertsql.py +++ b/lib/sqlalchemy/test/assertsql.py @@ -156,6 +156,8 @@ class CompiledSQL(SQLMatchRule): if not isinstance(params, list): params = [params] + all_params = list(params) + all_received = list(_received_parameters) while params: param = params.pop(0) if param not in _received_parameters: @@ -171,7 +173,7 @@ class CompiledSQL(SQLMatchRule): self._result = equivalent if not self._result: self._errmsg = "Testing for compiled statement %r partial params %r, " \ - "received %r with params %r" % (self.statement, params, _received_statement, _received_parameters) + "received %r with params %r" % (self.statement, all_params, _received_statement, all_received) class CountStatements(AssertRule): diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index e8b7d98370..120198ac59 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -320,7 +320,8 @@ class SingleCycleTest(UOWTest): sess.add_all([n2, n3]) sess.flush() - + + print "-------------------------------------------------" sess.delete(n1) sess.delete(n2) sess.delete(n3) @@ -437,6 +438,7 @@ class SingleCycleM2MTest(_base.MappedTest, testing.AssertsExecutionResults): n3.children = [n5, n4] sess.add_all([n1, n2, n3, n4, n5]) + self.assert_sql_execution( testing.db, sess.flush,