From 036cdbe7fb2d651ba5fbbc758c8584df011c8043 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 6 Apr 2018 11:39:15 -0400 Subject: [PATCH] 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 --- doc/build/changelog/unreleased_13/4232.rst | 10 +++ lib/sqlalchemy/orm/persistence.py | 8 +- lib/sqlalchemy/sql/sqltypes.py | 1 - test/orm/test_unitofwork.py | 89 +++++++++++++++++++++- 4 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 doc/build/changelog/unreleased_13/4232.rst diff --git a/doc/build/changelog/unreleased_13/4232.rst b/doc/build/changelog/unreleased_13/4232.rst new file mode 100644 index 0000000000..176650afd3 --- /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 4f1e08afac..a48bf9bf7e 100644 --- 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 c02ece98aa..573fda98fc 100644 --- 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 90616ae12b..8a4091ed42 100644 --- 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() -- 2.47.2