Raise informative exception for non-sortable PK

An informative exception is re-raised when a primary key value is not
sortable in Python during an ORM flush under Python 3, such as an ``Enum``
that has no ``__lt__()`` method; normally Python 3 raises a ``TypeError``
in this case.   The flush process sorts persistent objects by primary key
in Python so the values must be sortable.

Change-Id: Ia186968982dcd1234b82f2e701fefa2a1668a7e4
Fixes: #4232

diff --git a/doc/build/changelog/unreleased_13/4232.rst b/doc/build/changelog/unreleased_13/4232.rst
new file mode 100644 (file)
index 0000000..176650a
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4232.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4232
+
+    An informative exception is re-raised when a primary key value is not
+    sortable in Python during an ORM flush under Python 3, such as an ``Enum``
+    that has no ``__lt__()`` method; normally Python 3 raises a ``TypeError``
+    in this case.   The flush process sorts persistent objects by primary key
+    in Python so the values must be sortable.
+

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 4f1e08afacd20b40489b24ab1e068777b7bb64ff..a48bf9bf7ee31c617f9080721454e52e45b8bce1 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -1273,8 +1273,14 @@ def _sort_states(states):
     pending = set(states)
     persistent = set(s for s in pending if s.key is not None)
     pending.difference_update(persistent)
+    try:
+        persistent_sorted = sorted(persistent, key=lambda q: q.key[1])
+    except TypeError as err:
+        raise sa_exc.InvalidRequestError(
+            "Could not sort objects by primary key; primary key "
+            "values must be sortable in Python (was: %s)" % err)
     return sorted(pending, key=operator.attrgetter("insert_order")) + \
-        sorted(persistent, key=lambda q: q.key[1])
+        persistent_sorted
 
 
 class BulkUD(object):

diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py
index c02ece98aab839c0dce72c3d1daceedbbbef6a60..573fda98fc6ced86844bd6d87e5fc37f84c69560 100644 (file)
--- a/lib/sqlalchemy/sql/sqltypes.py
+++ b/lib/sqlalchemy/sql/sqltypes.py
@@ -1175,7 +1175,6 @@ class Enum(Emulated, String, SchemaType):
             two = 2
             three = 3
 
-
         t = Table(
             'data', MetaData(),
             Column('value', Enum(MyEnum))

diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py
index 90616ae12b5781f53af6b4070ef2e9e4fbde72ae..8a4091ed42ced6568aea850aaf3c7cd6359b1092 100644 (file)
--- a/test/orm/test_unitofwork.py
+++ b/test/orm/test_unitofwork.py
@@ -4,11 +4,12 @@
 from sqlalchemy.testing import eq_, assert_raises, assert_raises_message
 import datetime
 from sqlalchemy.orm import mapper as orm_mapper
+from sqlalchemy.util import OrderedDict
 
 import sqlalchemy as sa
-from sqlalchemy.util import u, ue, b
+from sqlalchemy.util import u, ue
 from sqlalchemy import Integer, String, ForeignKey, \
-    literal_column, event, Boolean, select, func
+    Enum, literal_column, event, Boolean, select, func
 from sqlalchemy import testing
 from sqlalchemy.testing.schema import Table
 from sqlalchemy.testing.schema import Column
@@ -2746,3 +2747,87 @@ class PartialNullPKTest(fixtures.MappedTest):
 
         t.col1 = "1"
         s.commit()
+
+
+class EnsurePKSortableTest(fixtures.MappedTest):
+    class SomeEnum(object):
+        # Implements PEP 435 in the minimal fashion needed by SQLAlchemy
+        __members__ = OrderedDict()
+
+        def __init__(self, name, value, alias=None):
+            self.name = name
+            self.value = value
+            self.__members__[name] = self
+            setattr(self.__class__, name, self)
+            if alias:
+                self.__members__[alias] = self
+                setattr(self.__class__, alias, self)
+
+    class MySortableEnum(SomeEnum):
+        __members__ = OrderedDict()
+
+        def __lt__(self, other):
+            return self.value < other.value
+
+    class MyNotSortableEnum(SomeEnum):
+        __members__ = OrderedDict()
+
+    one = MySortableEnum('one', 1)
+    two = MySortableEnum('two', 2)
+    three = MyNotSortableEnum('three', 3)
+    four = MyNotSortableEnum('four', 4)
+
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('t1', metadata,
+              Column('id', Enum(cls.MySortableEnum), primary_key=True),
+              Column('data', String(10)))
+
+        Table('t2', metadata,
+              Column('id', Enum(cls.MyNotSortableEnum), primary_key=True),
+              Column('data', String(10)))
+
+    @classmethod
+    def setup_classes(cls):
+        class T1(cls.Basic):
+            pass
+
+        class T2(cls.Basic):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        orm_mapper(cls.classes.T1, cls.tables.t1)
+        orm_mapper(cls.classes.T2, cls.tables.t2)
+
+    def test_exception_persistent_flush_py3k(self):
+        s = Session()
+
+        a, b = self.classes.T2(id=self.three), self.classes.T2(id=self.four)
+        s.add_all([a, b])
+        s.commit()
+
+        a.data = 'bar'
+        b.data = 'foo'
+        if sa.util.py3k:
+            assert_raises_message(
+                sa.exc.InvalidRequestError,
+                r"Could not sort objects by primary key; primary key values "
+                r"must be sortable in Python \(was: '<' not supported between "
+                r"instances of 'MyNotSortableEnum' and 'MyNotSortableEnum'\)",
+                s.flush
+            )
+        else:
+            s.flush()
+        s.close()
+
+    def test_persistent_flush_sortable(self):
+        s = Session()
+
+        a, b = self.classes.T1(id=self.one), self.classes.T1(id=self.two)
+        s.add_all([a, b])
+        s.commit()
+
+        a.data = 'bar'
+        b.data = 'foo'
+        s.commit()