- Fixed bug apparent only in Python 3 whereby

sorting of persistent + pending objects during
flush would produce an illegal comparison,
if the persistent object primary key
is not a single integer.  [ticket:2228]
Also in 0.6.9

diff --git a/CHANGES b/CHANGES
index 41f624af8d9dab4bbc52c4b85f20328e2a0dabf7..a7da519f18e23cfbe25e0f9d972b1ed493b76075 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -39,6 +39,13 @@ CHANGES
     [ticket:2224]  Thanks to the user who submitted
     the great test for this.
 
+  - Fixed bug apparent only in Python 3 whereby
+    sorting of persistent + pending objects during
+    flush would produce an illegal comparison,
+    if the persistent object primary key 
+    is not a single integer.  [ticket:2228]
+    Also in 0.6.9
+
   - Fixed bug whereby the source clause
     used by query.join() would be inconsistent
     if against a column expression that combined

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 8477f5e2f442c5a6c222fb9ca792f700ebf8d4c8..8a33ee376782ad8823b0a294ecb0ee6b7141c3ae 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2819,9 +2819,13 @@ def _event_on_resurrect(state):
 
 
 def _sort_states(states):
-    return sorted(states, key=operator.attrgetter('sort_key'))
-
-
+    ret = []
+    for haskey, g in groupby(states, key=lambda s:s.key is not None):
+        if haskey:
+            ret.extend(sorted(g, key=lambda st: st.key[1]))
+        else:
+            ret = sorted(g, key=operator.attrgetter("insert_order")) + ret
+    return ret
 
 class _ColumnMapping(util.py25_dict):
     """Error reporting helper for mapper._columntoproperty."""

diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index 0963a526bf7c63b1ceefc7f17aae39387728ebf3..76fddc6212122041936936f6953254fc5271f81d 100644 (file)
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -85,10 +85,6 @@ class InstanceState(object):
         else:
             return {}
 
-    @property
-    def sort_key(self):
-        return self.key and self.key[1] or (self.insert_order, )
-
     def initialize_instance(*mixed, **kwargs):
         self, instance, args = mixed[0], mixed[1], mixed[2:]
         manager = self.manager

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index f749783f21aef4fcd218855b4e4bfc608aa5c51e..c29a1c5182e3109e76ff0a6cde9c12c4ceca5ab2 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -11,6 +11,7 @@ from sqlalchemy.orm import mapper, relationship, backref, \
     validates, aliased, defer, deferred, synonym, attributes, \
     column_property, composite, dynamic_loader, \
     comparable_property, Session
+from sqlalchemy.orm.mapper import _sort_states
 from test.lib.testing import eq_, AssertsCompiledSQL
 from test.lib import fixtures
 from test.orm import _fixtures
@@ -110,7 +111,7 @@ class MapperTest(_fixtures.FixtureTest):
         })
 
         # test that QueryableAttribute.__str__() doesn't 
-        # cause a compile.  
+        # cause a compile.
         eq_(str(User.addresses), "User.addresses")
 
     def test_exceptions_sticky(self):
@@ -241,6 +242,43 @@ class MapperTest(_fixtures.FixtureTest):
         assert_raises(TypeError, Foo, x=5)
         assert_raises(TypeError, Bar, x=5)
 
+    def test_sort_states_comparisons(self):
+        """test that _sort_states() doesn't compare 
+        insert_order to state.key, for set of mixed
+        persistent/pending.  In particular Python 3 disallows
+        this.  
+        
+        """
+        class Foo(object):
+            def __init__(self, id):
+                self.id = id
+        m = MetaData()
+        foo_t = Table('foo', m, 
+                        Column('id', String, primary_key=True)
+                    )
+        m = mapper(Foo, foo_t)
+        class DontCompareMeToString(int):
+        # Py3K
+        #    pass
+        # Py2K
+            def __lt__(self, other):
+                assert not isinstance(other, basestring)
+                return int(self) < other
+        # end Py2K
+        foos = [Foo(id='f%d' % i) for i in range(5)]
+        states = [attributes.instance_state(f) for f in foos]
+
+        for s in states[0:3]:
+            s.key = m._identity_key_from_state(s)
+        states[3].insert_order = DontCompareMeToString(5)
+        states[4].insert_order = DontCompareMeToString(1)
+        states[2].insert_order = DontCompareMeToString(3)
+        eq_(
+            _sort_states(states),
+            [states[4], states[3], states[0], states[1], states[2]]
+        )
+
+
     def test_props(self):
         users, Address, addresses, User = (self.tables.users,
                                 self.classes.Address,
@@ -2207,7 +2245,7 @@ class DeferredTest(_fixtures.FixtureTest):
     def test_map_selectable_wo_deferred(self):
         """test mapping to a selectable with deferred cols,
         the selectable doesn't include the deferred col.
-        
+
         """
 
         Order, orders = self.classes.Order, self.tables.orders