Another case of illegal sorting in Python 3

[sqlalchemy-bot]

Migrated issue, originally created by Chris Wilson (@qris1)

Similar to #2228, but this one occurs when removing multiple objects from a session (deleting from the database) in a single commit, and the primary key contains an object which has no sort order defined, such as an Enum.

If there are multiple persistent objects to be deleted, then _sort_states in persistence.py wants to sort them, and it uses the identity key to do so. But if it contains non-comparable objects, this will fail.

I'm not sure exactly why we want to "sort the states" here. But if the sort order is arbitrary then we could just sort by ID (or not sort them at all?).

I've seen this on Postgres 10 and SQLite (demo below).

from sqlalchemy import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Session

import enum
class MyEnum(enum.Enum):
    one = 1
    two = 2
    three = 3
    
Base = declarative_base()

class A(Base):
    __tablename__ = 'a'

    id = Column(Enum(MyEnum), primary_key=True)
    data = Column(Integer)

e = create_engine('sqlite://', echo=True)
Base.metadata.create_all(e)

s = Session(e)

a1 = A(id=MyEnum.one)
s.add(a1)
s.commit()

a2 = A(id=MyEnum.two)
s.add(a2)
s.commit()

s.delete(a1)
s.delete(a2)
s.commit()

The resulting error is:

File untitled1.py, line 45, in : s.commit() 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\session.py, line 906, in commit : self.transaction.commit() 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\session.py, line 461, in commit : self._prepare_impl() 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\session.py, line 441, in _prepare_impl : self.session.flush() 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\session.py, line 2177, in flush : self._flush(objects) 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\session.py, line 2297, in _flush : transaction.rollback(_capture_exception=True) 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\util\langhelpers.py, line 66, in __exit__ : compat.reraise(exc_type, exc_value, exc_tb) 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\util\compat.py, line 187, in reraise : raise value 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\session.py, line 2261, in _flush : flush_context.execute() 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\unitofwork.py, line 389, in execute : rec.execute(self) 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\unitofwork.py, line 577, in execute : uow 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\persistence.py, line 243, in delete_obj : uowtransaction)) 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\persistence.py, line 357, in _organize_states_for_delete : states): 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\persistence.py, line 1108, in _connections_for_states : for state in _sort_states(states): 
File sqlalchemy-1.1.14-py3.6-win-amd64.egg\sqlalchemy\orm\persistence.py, line 1130, in _sort_states : sorted(persistent, key=lambda q: q.key[1]) 
TypeError: '<' not supported between instances of 'MyEnum' and 'MyEnum'

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

I thought I had this in the docs but it may have been removed, primary key values must be comparable. ill try to look to see why that's no longer in the docs (and confirm that it was, at some point)

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

this will raise a nicer exception, but doesn't "fix" anything for you, you need to put an __lt__() method on your Enum.

https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/721

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• set milestone to "1.3"

[sqlalchemy-bot]

Chris Wilson (@qris1) wrote:

Thanks for the quick response! I couldn't find anything about this restriction in the docs. Would it be hard to remove the need for sortability?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

I'm not sure exactly why we want to "sort the states" here.

that's another thing I've definitely written about and can now find nowhere, when you run a bunch of UPDATE statements you want the ordering of the rows to be deterministic to minimize the chance of deadlocks with other transactions. Else if transaction A wants to update row 5 then row 7, transaction B wants to update row 7 then 5, deadlock.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Would it be hard to remove the need for sortability?

any database primary key value is sortable because it has to be indexed so it makes sense your in-python object should be sortable in some way, just stick an __lt__() method on it.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

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

→ 036cdbe

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed