don't need id(obj) in topological anymore

Issue #1756 resolved
Mike Bayer repo owner created an issue

while the UOW is being vastly simplified for 0.7, it seems like we don't need the id() call in topological anymore. Verify that the only two objects that ever get passed to topological as nodes are Table and InstanceState, and that there's no weird issue in Jython or whatever preventing those from being hashable - this appears to be the case so far.

diff -r 1258d1ed9e8064e08fd77d2dc3938fcebc130800 lib/sqlalchemy/sql/util.py
--- a/lib/sqlalchemy/sql/util.py    Wed Mar 31 13:52:57 2010 -0400
+++ b/lib/sqlalchemy/sql/util.py    Wed Mar 31 17:43:36 2010 -0400
@@ -15,7 +15,8 @@
         parent_table = fkey.column.table
         if parent_table in tables:
             child_table = fkey.parent.table
-            tuples.append( ( parent_table, child_table ) )
+            if parent_table is not child_table:
+                tuples.append((parent_table, child_table))

     for table in tables:
         visitors.traverse(table, {'schema_visitor':True}, {'foreign_key':visit_foreign_key})    
diff -r 1258d1ed9e8064e08fd77d2dc3938fcebc130800 lib/sqlalchemy/topological.py
--- a/lib/sqlalchemy/topological.py Wed Mar 31 13:52:57 2010 -0400
+++ b/lib/sqlalchemy/topological.py Wed Mar 31 17:43:36 2010 -0400
@@ -161,21 +161,20 @@
     edges = _EdgeCollection()

     for item in list(allitems) + [t[0](t[0) for t in tuples] + [t[1](t[1) for t in tuples]:
-        item_id = id(item)
-        if item_id not in nodes:
-            nodes[item_id](item_id) = _Node(item)
+        if item not in nodes:
+            nodes[item](item) = _Node(item)

     for t in tuples:
-        id0, id1 = id(t[0](0)), id(t[1](1))
-        if t[0](0) is t[1](1):
+        t0, t1 = t[0](0), t[1](1)
+        if t0 is t1:
             if allow_cycles:
-                n = nodes[id0](id0)
+                n = nodes[t0](t0)
                 n.cycles = set([n](n))
             elif not ignore_self_cycles:
                 raise CircularDependencyError("Self-referential dependency detected: %r" % t)
             continue
-        childnode = nodes[id1](id1)
-        parentnode = nodes[id0](id0)
+        childnode = nodes[t1](t1)
+        parentnode = nodes[t0](t0)
         edges.add((parentnode, childnode))

     queue = [-198,6 +197,7 @@
                         lead.cycles.add(edge[1](]
@@))
                         if n is not None:
                             queue.append(n)
+                            
                     for n in lead.cycles:
                         if n is not lead:
                             n._cyclical = True
@@ -211,7 +211,7 @@
         node = queue.pop()
         if not hasattr(node, '_cyclical'):
             output.append(node)
-        del nodes[id(node.item)](id(node.item))
+        del nodes[node.item](node.item)
         for childnode in edges.pop_node(node):
             queue.append(childnode)
     return output

Comments (3)

  1. Mike Bayer reporter

    confirmed, the four types of objects passed in are Table, InstanceState, Mapper, MapperStub, all of which are hashable.

  2. Log in to comment