- 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]

diff --git a/CHANGES b/CHANGES
index 7feaca8cc0c695e5c26548fa603cbc7ef75ca938..e00f78e0a16f12e8791d735ea9d8120f73ef21cf 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -11,6 +11,12 @@ CHANGES
     if against a column expression that combined
     multiple entities together.  [ticket:2197]
 
+  - 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]
+
   - Fixed bug whereby if a mapped class
     redefined __hash__() or __eq__() to something
     non-standard, which is a supported use case

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index de0023ddf7db5f2be531cc26a095bd25289492b5..d2d78dd39cb0212ebd098e97b281fd394af15c54 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2447,7 +2447,13 @@ def _event_on_resurrect(state, instance):
 
 
 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
 
 def _load_scalar_attributes(state, attribute_names):
     """initiate a column-based attribute refresh operation."""

diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index c7dda8a4778cc0a82cb692cb9e5f66926224f456..2174d562d8c75ca75bd723cd5020e2fc853bdfd1 100644 (file)
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -88,10 +88,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 be3903b7dfa60fa2829a0bb9b513f95fc21e078d..70e5865b3d829b372bfa4fbfde3958a523295bf4 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -14,6 +14,7 @@ from sqlalchemy.orm import defer, deferred, synonym, attributes, \
     comparable_property, AttributeExtension, Session
 from sqlalchemy.test.testing import eq_, AssertsCompiledSQL
 from test.orm import _base, _fixtures
+from sqlalchemy.orm.mapper import _sort_states
 from sqlalchemy.test.assertsql import AllOf, CompiledSQL
 import logging
 
@@ -236,6 +237,42 @@ 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]]
+        )
+
     @testing.resolve_artifact_names
     def test_props(self):
         m = mapper(User, users, properties = {