From: Mike Bayer Date: Thu, 8 Apr 2010 01:00:16 +0000 (-0400) Subject: - make it exceedlingly obvious that all topological/unitofwork code is X-Git-Tag: rel_0_6_0~27^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8967b1153a6776870f0b48175943c1b05133bfc2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - make it exceedlingly obvious that all topological/unitofwork code is being rewritten, and nothing here should be consulted for any future activity. - underscore current topological methods as their API behavior is changing, possibly in 0.6.1 if [ticket:1742] remains on track --- diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index f2959d1414..1ec22127c9 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -6,10 +6,13 @@ """Relationship dependencies. -Bridges the ``PropertyLoader`` (i.e. a ``relationship()``) and the +Bridges the ``RelationshipLoader`` (i.e. a ``relationship()``) and the ``UOWTransaction`` together to allow processing of relationship()-based dependencies at flush time. +A large portion of this module will be reworked in an +upcoming release. See [ticket:1742] for details. + """ from sqlalchemy import sql, util @@ -546,11 +549,8 @@ class ManyToManyDP(DependencyProcessor): class MapperStub(object): """Represent a many-to-many dependency within a flush context. - - The UOWTransaction corresponds dependencies to mappers. - MapperStub takes the place of the "association table" - so that a depedendency can be corresponded to it. - + + This object is deprecated. """ def __init__(self, parent, mapper, key): diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 7fbb0862d0..1a82b96b1d 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -510,6 +510,8 @@ class MapperProperty(object): Establishes a topological dependency between two mappers which will affect the order in which mappers persist data. + This method is deprecated. + """ pass @@ -518,10 +520,11 @@ class MapperProperty(object): """Called by the ``Mapper`` in response to the UnitOfWork calling the ``Mapper``'s register_processors operation. Establishes a processor object between two mappers which - will link data and state between parent/child objects. + will synchronize state between parent/child objects. + + This method is deprecated. """ - pass def is_primary(self): diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 30b0b61e5e..ea4e0b1dad 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -4,19 +4,17 @@ # This module is part of SQLAlchemy and is released under # the MIT License: http://www.opensource.org/licenses/mit-license.php -"""The internals for the Unit Of Work system. +"""The internals for the unit of work system. -Includes hooks into the attributes package enabling the routing of -change events to Unit Of Work objects, as well as the flush() -mechanism which creates a dependency structure that executes change -operations. +The session's flush() process passes objects to a contextual object +here, which assembles flush tasks based on mappers and their properties, +organizes them in order of dependency, and executes. + +Most of the code in this module is obsolete, and will +be replaced by a much simpler and more efficient +system in an upcoming release. See [ticket:1742] for +details. -A Unit of Work is essentially a system of maintaining a graph of -in-memory objects and their modified state. Objects are maintained as -unique against their primary key identity using an *identity map* -pattern. The Unit of Work then maintains lists of objects that are -new, dirty, or deleted and provides the capability to flush all those -changes at once. """ @@ -40,7 +38,8 @@ class UOWEventHandler(interfaces.AttributeExtension): self.key = key def append(self, state, item, initiator): - # process "save_update" cascade rules for when an instance is appended to the list of another instance + # process "save_update" cascade rules for when + # an instance is appended to the list of another instance sess = _state_session(state) if sess: prop = _state_mapper(state).get_property(self.key) @@ -74,15 +73,8 @@ class UOWEventHandler(interfaces.AttributeExtension): class UOWTransaction(object): - """Handles the details of organizing and executing transaction - tasks during a UnitOfWork object's flush() operation. - - The central operation is to form a graph of nodes represented by the - ``UOWTask`` class, which is then traversed by a ``UOWExecutor`` object - that issues SQL and instance-synchronizing operations via the related - packages. - """ - + """Represent the state of a flush operation in progress.""" + def __init__(self, session): self.session = session self.mapper_flush_opts = session._mapper_flush_opts @@ -94,7 +86,8 @@ class UOWTransaction(object): # dictionary of mappers to UOWTasks self.tasks = {} - # dictionary used by external actors to store arbitrary state + # dictionary used by external actors + # to store arbitrary state # information. self.attributes = {} @@ -169,8 +162,6 @@ class UOWTransaction(object): def get_task_by_mapper(self, mapper, dontcreate=False): """return UOWTask element corresponding to the given mapper. - Will create a new UOWTask, including a UOWTask corresponding to the - "base" inherited mapper, if needed, unless the dontcreate flag is True. """ try: @@ -195,16 +186,13 @@ class UOWTransaction(object): return task def register_dependency(self, mapper, dependency): - """register a dependency between two mappers. - - Called by ``mapper.PropertyLoader`` to register the objects - handled by one mapper being dependent on the objects handled - by another. - - """ + """register a dependency between two mappers.""" + # correct for primary mapper - # also convert to the "base mapper", the parentmost task at the top of an inheritance chain - # dependency sorting is done via non-inheriting mappers only, dependencies between mappers + # also convert to the "base mapper", the parentmost + # task at the top of an inheritance chain + # dependency sorting is done via non-inheriting + # mappers only, dependencies between mappers # in the same inheritance chain is done at the per-object level mapper = mapper.primary_mapper().base_mapper dependency = dependency.primary_mapper().base_mapper @@ -226,14 +214,7 @@ class UOWTransaction(object): task.dependencies.add(up) def execute(self): - """Execute this UOWTransaction. - - This will organize all collected UOWTasks into a dependency-sorted - list which is then traversed using the traversal scheme - encoded in the UOWExecutor class. Operations to mappers and dependency - processors are fired off in order to issue SQL to the database and - synchronize instance attributes with database values and related - foreign key values.""" + """Execute this steps assembled into this UOWTransaction.""" # pre-execute dependency processors. this process may # result in new tasks, objects and/or dependency processors being added, @@ -281,7 +262,7 @@ class UOWTransaction(object): self.session._register_newly_persistent(elem.state) def _sort_dependencies(self): - nodes = topological.sort_with_cycles(self.dependencies, + nodes = topological._sort_with_cycles(self.dependencies, [t.mapper for t in self.tasks.itervalues() if t.base_task is t] ) @@ -302,7 +283,10 @@ class UOWTransaction(object): log.class_logger(UOWTransaction) class UOWTask(object): - """A collection of mapped states corresponding to a particular mapper.""" + """A collection of mapped states corresponding to a particular mapper. + + This object is deprecated. + """ def __init__(self, uowtransaction, mapper, base_task=None): self.uowtransaction = uowtransaction @@ -339,30 +323,6 @@ class UOWTask(object): """Return an iterator of UOWTask objects corresponding to the inheritance sequence of this UOWTask's mapper. - e.g. if mapper B and mapper C inherit from mapper A, and - mapper D inherits from B: - - mapperA -> mapperB -> mapperD - -> mapperC - - the inheritance sequence starting at mapper A is a depth-first - traversal: - - [mapperA, mapperB, mapperD, mapperC] - - this method will therefore return - - [UOWTask(mapperA), UOWTask(mapperB), UOWTask(mapperD), - UOWTask(mapperC)] - - The concept of "polymporphic iteration" is adapted into - several property-based iterators which return object - instances, UOWTaskElements and UOWDependencyProcessors in an - order corresponding to this sequence of parent UOWTasks. This - is used to issue operations related to inheritance-chains of - mappers in the proper order based on dependencies between - those mappers. - """ for mapper in self.inheriting_mappers: t = self.base_task._inheriting_tasks.get(mapper, None) @@ -370,9 +330,9 @@ class UOWTask(object): yield t def is_empty(self): - """return True if this UOWTask is 'empty', meaning it has no child items. + """return True if this UOWTask is 'empty', + meaning it has no child items. - used only for debugging output. """ return not self._objects and not self.dependencies @@ -386,19 +346,17 @@ class UOWTask(object): rec.update(listonly, isdelete) def append_postupdate(self, state, post_update_cols): - """issue a 'post update' UPDATE statement via this object's mapper immediately. + """issue a 'post update' UPDATE statement via + this object's mapper immediately. - this operation is used only with relationships that specify the `post_update=True` - flag. """ - # postupdates are UPDATED immeditely (for now) - # convert post_update_cols list to a Set so that __hash__() is used to compare columns - # instead of __eq__() - self.mapper._save_obj([state], self.uowtransaction, postupdate=True, post_update_cols=set(post_update_cols)) + self.mapper._save_obj([state], self.uowtransaction, + postupdate=True, post_update_cols=set(post_update_cols)) def __contains__(self, state): - """return True if the given object is contained within this UOWTask or inheriting tasks.""" + """return True if the given object is contained + within this UOWTask or inheriting tasks.""" for task in self.polymorphic_tasks: if state in task._objects: @@ -407,7 +365,8 @@ class UOWTask(object): return False def is_deleted(self, state): - """return True if the given object is marked as to be deleted within this UOWTask.""" + """return True if the given object is marked + as to be deleted within this UOWTask.""" try: return self._objects[state].isdelete @@ -477,10 +436,15 @@ class UOWTask(object): return self.cyclical_dependencies def _sort_circular_dependencies(self, trans, cycles): - """Topologically sort individual entities with row-level dependencies. + """sort row-level dependencies. - Builds a modified UOWTask structure, and is invoked when the - per-mapper topological structure is found to have cycles. + Note that this method is a total disaster, as it was + bolted onto the originally simple unit-of-work + system after more complex mappings revealed + the presence of inter-row rependencies - this occured + well within version 0.1 and despite many fixes + has remained the most legacy code within SQLAlchemy. + It is gone without a trace after [ticket:1742]. """ @@ -492,7 +456,8 @@ class UOWTask(object): if depprocessor not in tasks: tasks[depprocessor] = UOWDependencyProcessor( depprocessor.processor, - UOWTask(self.uowtransaction, depprocessor.targettask.mapper) + UOWTask(self.uowtransaction, + depprocessor.targettask.mapper) ) tasks[depprocessor].targettask.append(target_state, isdelete=isdelete) @@ -529,7 +494,8 @@ class UOWTask(object): isdelete = taskelement.isdelete # list of dependent objects from this object - (added, unchanged, deleted) = dep.get_object_dependencies(state, trans, passive=True) + (added, unchanged, deleted) = dep.get_object_dependencies( + state, trans, passive=True) if not added and not unchanged and not deleted: continue @@ -551,9 +517,11 @@ class UOWTask(object): tuples.append(whosdep) if whosdep[0] is state: - set_processor_for_state(whosdep[0], dep, whosdep[0], isdelete=isdelete) + set_processor_for_state(whosdep[0], dep, whosdep[0], + isdelete=isdelete) else: - set_processor_for_state(whosdep[0], dep, whosdep[1], isdelete=isdelete) + set_processor_for_state(whosdep[0], dep, whosdep[1], + isdelete=isdelete) else: # TODO: no test coverage here set_processor_for_state(state, dep, state, isdelete=isdelete) @@ -567,11 +535,7 @@ class UOWTask(object): # dependency - keep non-dependent objects # grouped together, so that insert ordering as determined # by session.add() is maintained. - # An alternative might be to represent the "insert order" - # as part of the topological sort itself, which would - # eliminate the need for this step (but may make the original - # topological sort more expensive) - head = topological.sort_as_tree(tuples, object_to_original_task.iterkeys()) + head = topological._sort_as_tree(tuples, object_to_original_task.iterkeys()) if head is not None: original_to_tasks = {} stack = [(head, t)] @@ -588,7 +552,8 @@ class UOWTask(object): else: task = original_to_tasks[(parenttask, originating_task)] - task.append(state, originating_task._objects[state].listonly, isdelete=originating_task._objects[state].isdelete) + task.append(state, originating_task._objects[state].listonly, + isdelete=originating_task._objects[state].isdelete) if state in dependencies: task.cyclical_dependencies.update(dependencies[state].itervalues()) @@ -614,10 +579,9 @@ class UOWTask(object): return ("UOWTask(%s) Mapper: '%r'" % (hex(id(self)), self.mapper)) class UOWTaskElement(object): - """Corresponds to a single InstanceState to be saved, deleted, - or otherwise marked as having dependencies. A collection of - UOWTaskElements are held by a UOWTask. - + """Represent a single state to be saved. + + This object is deprecated. """ def __init__(self, state): self.state = state @@ -642,11 +606,9 @@ class UOWTaskElement(object): ) class UOWDependencyProcessor(object): - """In between the saving and deleting of objects, process - dependent data, such as filling in a foreign key on a child item - from a new primary key, or deleting association rows before a - delete. This object acts as a proxy to a DependencyProcessor. + """Represent tasks in between inserts/updates/deletes. + This object is deprecated. """ def __init__(self, processor, targettask): self.processor = processor @@ -674,19 +636,8 @@ class UOWDependencyProcessor(object): return hash((self.processor, self.targettask)) def preexecute(self, trans): - """preprocess all objects contained within this ``UOWDependencyProcessor``s target task. - - This may locate additional objects which should be part of the - transaction, such as those affected deletes, orphans to be - deleted, etc. - - Once an object is preprocessed, its ``UOWTaskElement`` is marked as processed. If subsequent - changes occur to the ``UOWTaskElement``, its processed flag is reset, and will require processing - again. - - Return True if any objects were preprocessed, or False if no - objects were preprocessed. If True is returned, the parent ``UOWTransaction`` will - ultimately call ``preexecute()`` again on all processors until no new objects are processed. + """preprocess all objects contained within + this ``UOWDependencyProcessor``s target task. """ def getobj(elem): @@ -710,7 +661,8 @@ class UOWDependencyProcessor(object): return ret def execute(self, trans, delete): - """process all objects contained within this ``UOWDependencyProcessor``s target task.""" + """process all objects contained within this + ``UOWDependencyProcessor``s target task.""" elements = [e for e in @@ -729,15 +681,18 @@ class UOWDependencyProcessor(object): def whose_dependent_on_who(self, state1, state2): """establish which object is operationally dependent amongst a parent/child using the semantics stated by the dependency processor. - - This method is used to establish a partial ordering (set of dependency tuples) - when toplogically sorting on a per-instance basis. - """ return self.processor.whose_dependent_on_who(state1, state2) class UOWExecutor(object): - """Encapsulates the execution traversal of a UOWTransaction structure.""" + """Encapsulates the execution traversal + of a UOWTransaction structure. + + This part of the approach is the core flaw that's + being removed with [ticket:1742], as it necessitates + deep levels of recursion. + + """ def execute(self, trans, tasks, isdelete=None): if isdelete is not True: diff --git a/lib/sqlalchemy/orm/uowdumper.py b/lib/sqlalchemy/orm/uowdumper.py index dd96b6b9aa..7884cabd43 100644 --- a/lib/sqlalchemy/orm/uowdumper.py +++ b/lib/sqlalchemy/orm/uowdumper.py @@ -4,7 +4,12 @@ # This module is part of SQLAlchemy and is released under # the MIT License: http://www.opensource.org/licenses/mit-license.php -"""Dumps out a string representation of a UOWTask structure""" +"""Dumps out a string representation of a UOWTask structure. + +This module is deprecated and will be removed once +[ticket:1742] is complete. + +""" from sqlalchemy.orm import unitofwork from sqlalchemy.orm import util as mapperutil diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 5a439b099d..ccbeea3712 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -20,7 +20,7 @@ def sort_tables(tables): for table in tables: visitors.traverse(table, {'schema_visitor':True}, {'foreign_key':visit_foreign_key}) - return topological.sort(tuples, tables) + return topological._sort(tuples, tables) def find_join_source(clauses, join_to): """Given a list of FROM clauses and a selectable, diff --git a/lib/sqlalchemy/topological.py b/lib/sqlalchemy/topological.py index d061aec043..cfc07e861d 100644 --- a/lib/sqlalchemy/topological.py +++ b/lib/sqlalchemy/topological.py @@ -6,57 +6,46 @@ """Topological sorting algorithms. -The topological sort is an algorithm that receives this list of -dependencies as a *partial ordering*, that is a list of pairs which -might say, *X is dependent on Y*, *Q is dependent on Z*, but does not -necessarily tell you anything about Q being dependent on X. Therefore, -its not a straight sort where every element can be compared to -another... only some of the elements have any sorting preference, and -then only towards just some of the other elements. For a particular -partial ordering, there can be many possible sorts that satisfy the -conditions. +All functions and classes in this module are currently deprecated, +and will be replaced by a much simpler and more efficient +system in an upcoming release. See [ticket:1742] for +details. """ from sqlalchemy.exc import CircularDependencyError from sqlalchemy import util -__all__ = ['sort', 'sort_with_cycles', 'sort_as_tree'] - -def sort(tuples, allitems): +def _sort(tuples, allitems): """sort the given list of items by dependency. - 'tuples' is a list of tuples representing a partial ordering. + deprecated. a new sort with slightly different + behavior will replace this method in an upcoming release. """ - return [n.item for n in _sort(tuples, allitems, allow_cycles=False, ignore_self_cycles=True)] + return [n.item for n in _sort_impl(tuples, allitems, + allow_cycles=False, + ignore_self_cycles=True)] -def sort_with_cycles(tuples, allitems): +def _sort_with_cycles(tuples, allitems): """sort the given list of items by dependency, cutting out cycles. - - returns results as an iterable of 2-tuples, containing the item, - and a list containing items involved in a cycle with this item, if any. - - 'tuples' is a list of tuples representing a partial ordering. + + deprecated. a new approach to cycle detection will + be introduced in an upcoming release. """ - return [(n.item, [n.item for n in n.cycles or []]) for n in _sort(tuples, allitems, allow_cycles=True)] + return [(n.item, [n.item for n in n.cycles or []]) + for n in _sort_impl(tuples, allitems, allow_cycles=True)] -def sort_as_tree(tuples, allitems, with_cycles=False): +def _sort_as_tree(tuples, allitems, with_cycles=False): """sort the given list of items by dependency, and return results as a hierarchical tree structure. - returns results as an iterable of 3-tuples, containing the item, - a list containing items involved in a cycle with this item, if any, - and a list of child tuples. - - if with_cycles is False, the returned structure is of the same form - but the second element of each tuple, i.e. the 'cycles', is an empty list. - - 'tuples' is a list of tuples representing a partial ordering. + deprecated. a new approach to "grouped" topological sorting + will be introduced in an upcoming release. """ - return _organize_as_tree(_sort(tuples, allitems, allow_cycles=with_cycles)) + return _organize_as_tree(_sort_impl(tuples, allitems, allow_cycles=with_cycles)) class _Node(object): @@ -156,7 +145,7 @@ class _EdgeCollection(object): def __repr__(self): return repr(list(self)) -def _sort(tuples, allitems, allow_cycles=False, ignore_self_cycles=False): +def _sort_impl(tuples, allitems, allow_cycles=False, ignore_self_cycles=False): nodes = {} edges = _EdgeCollection() @@ -221,6 +210,8 @@ def _organize_as_tree(nodes): set as siblings to each other as possible. returns nodes as 3-tuples (item, cycles, children). + + this function is deprecated. """ if not nodes: @@ -263,6 +254,9 @@ def _organize_as_tree(nodes): return (head.item, [n.item for n in head.cycles or []], head.children) def _find_cycles(edges): + """ + this function is deprecated. + """ cycles = {} def traverse(node, cycle, goal): diff --git a/test/base/test_dependency.py b/test/base/test_dependency.py index 890dd76078..bd7d811129 100644 --- a/test/base/test_dependency.py +++ b/test/base/test_dependency.py @@ -1,4 +1,4 @@ -import sqlalchemy.topological as topological +from sqlalchemy import topological from sqlalchemy.test import TestBase @@ -56,7 +56,7 @@ class DependencySortTest(TestBase): (node4, subnode3), (node4, subnode4) ] - head = topological.sort_as_tree(tuples, []) + head = topological._sort_as_tree(tuples, []) self.assert_sort(tuples, head) def testsort2(self): @@ -74,7 +74,7 @@ class DependencySortTest(TestBase): (node5, node6), (node6, node2) ] - head = topological.sort_as_tree(tuples, [node7]) + head = topological._sort_as_tree(tuples, [node7]) self.assert_sort(tuples, head, [node7]) def testsort3(self): @@ -87,9 +87,9 @@ class DependencySortTest(TestBase): (node3, node2), (node1,node3) ] - head1 = topological.sort_as_tree(tuples, [node1, node2, node3]) - head2 = topological.sort_as_tree(tuples, [node3, node1, node2]) - head3 = topological.sort_as_tree(tuples, [node3, node2, node1]) + head1 = topological._sort_as_tree(tuples, [node1, node2, node3]) + head2 = topological._sort_as_tree(tuples, [node3, node1, node2]) + head3 = topological._sort_as_tree(tuples, [node3, node2, node1]) # TODO: figure out a "node == node2" function #self.assert_(str(head1) == str(head2) == str(head3)) @@ -108,7 +108,7 @@ class DependencySortTest(TestBase): (node1, node3), (node3, node2) ] - head = topological.sort_as_tree(tuples, []) + head = topological._sort_as_tree(tuples, []) self.assert_sort(tuples, head) def testsort5(self): @@ -131,7 +131,7 @@ class DependencySortTest(TestBase): node3, node4 ] - head = topological.sort_as_tree(tuples, allitems, with_cycles=True) + head = topological._sort_as_tree(tuples, allitems, with_cycles=True) self.assert_sort(tuples, head) def testcircular(self): @@ -149,7 +149,7 @@ class DependencySortTest(TestBase): (node4, node1) ] allitems = [node1, node2, node3, node4] - head = topological.sort_as_tree(tuples, allitems, with_cycles=True) + head = topological._sort_as_tree(tuples, allitems, with_cycles=True) self.assert_sort(tuples, head) def testcircular2(self): @@ -166,7 +166,7 @@ class DependencySortTest(TestBase): (node3, node2), (node2, node3) ] - head = topological.sort_as_tree(tuples, [], with_cycles=True) + head = topological._sort_as_tree(tuples, [], with_cycles=True) self.assert_sort(tuples, head) def testcircular3(self): @@ -174,16 +174,16 @@ class DependencySortTest(TestBase): tuples = [(question, issue), (providerservice, issue), (provider, question), (question, provider), (providerservice, question), (provider, providerservice), (question, answer), (issue, question)] - head = topological.sort_as_tree(tuples, [], with_cycles=True) + head = topological._sort_as_tree(tuples, [], with_cycles=True) self.assert_sort(tuples, head) def testbigsort(self): tuples = [(i, i + 1) for i in range(0, 1500, 2)] - head = topological.sort_as_tree(tuples, []) + head = topological._sort_as_tree(tuples, []) def testids(self): # ticket:1380 regression: would raise a KeyError - topological.sort([(id(i), i) for i in range(3)], []) + topological._sort([(id(i), i) for i in range(3)], [])