Should Country be orderable?

[TrilceAC]

Suppose that your models must support Persons which might have many Nationalities and you decide to proceed using a Person class and another Nationality class to represent the nationalities of persons by means of a foreign key column to the person id and a CountryType column to represent the related nacionality:

class Persona(Model):
    id = Column(Integer, primary_key=True, index=True)
    name = Column(Unicode(35), nullable=False, index=True)
    surname = Column(Unicode(35), nullable=False, index=True)

class Nationality(Model):
    person_id = Column(Integer,
                        ForeignKey('person.id',
                                   ondelete='CASCADE'),
                        primary_key=True,
                        index=True)

    person = relationship(
        'Person',
        backref=backref(
            'nationalities',
            cascade='all, delete-orphan'
        )
    )

    country = Column(CountryType, primary_key=True, index=True)

No orphan is desired, therefore ondelete='CASCADE' is used on the foreign key, as well as cascade='all, delete-orphan' is used on the relationship.

Suppose that john is added and commited:

>>>john = Person(name='John', surname='Doe', nationalities=[Nationality(country='ES'), Nationality(country='EU')])
>>>db.session.add(john)
>>>db.session.commit()

At this point, if one tries to delete john, the behaviour would depend on whether the object is loaded on the session or not. If the object is not loaded, the deletion will succeed, because the RDBMS is in charge of deleting the related rows and it knows how to do that:

>>>db.session.delete(john)
>>>db.session.commit()

But if the object is loaded in the session, a TypeError: unorderable types: Country() < Country() exception raises:

>>># Remember to recreate john if you deleted it!!!
>>>john
<Person(3, 'John Doe')>
>>># Calling john loads it from the database.
>>> db.session.delete(john)
>>> db.session.commit()

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/scoping.py", line 153, in do
    return getattr(self.registry(), name)(*args, **kwargs)
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 943, in commit
    self.transaction.commit()
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 467, in commit
    self._prepare_impl()
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 447, in _prepare_impl
    self.session.flush()
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 2254, in flush
    self._flush(objects)
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 2381, in _flush
    transaction.rollback(_capture_exception=True)
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/util/langhelpers.py", line 66, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/util/compat.py", line 249, in reraise
    raise value
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/session.py", line 2345, in _flush
    flush_context.execute()
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/unitofwork.py", line 395, in execute
    rec.execute(self)
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/unitofwork.py", line 597, in execute
    uow
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 250, in delete_obj
    uowtransaction))
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 364, in _organize_states_for_delete
    states):
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 1258, in _connections_for_states
    for state in _sort_states(states):
  File ".virtualenvs/gesific/lib/python3.5/site-packages/sqlalchemy/orm/persistence.py", line 1280, in _sort_states
    sorted(persistent, key=lambda q: q.key[1])
TypeError: unorderable types: Country() < Country()

It seems that SQLAlchemy needs to know how to order Country objects, but Country is not orderable. Should methods like __lt__ be implemented in Country? Which criteria should be used? Maybe alphabetically comparing Country.code?

[kvesteri]

Ordering by code makes sense. You can use total_ordering decorator. PRs welcome

[TrilceAC]

Digging a bit further, I have seen what is the problem, and I have found two feasible solutions:

The first one is, as previously proposed, to implement Country.__lt__(self, other):

@str_coercible
class Country(object):
    # ...
    def __lt__(self, other):
        if isinstance(other, Country):
            return self.code < other.code
        if isinstance(other, str):
            return self.code < other

This is a way to solve the problem, but it adds some semantics that might be completely undesired.

The cause of the problem is that country is part of the primary key of Nationality. Nationality was intended to be a table to represent a many-to-may relationship but without doing a foreign key on the country field. On country I wanted just to store the value, which is enough data and compact enough. Instead of using an id column to represented the primary key, I decided to make use of a composed primary key, being country part of it. Since SQLAlchemy needs to be able to order objects by primary key, country needed to be orderable. Changing the model in a way that uses an alternative and orderable primary key resolves the issue too:

In this second solution, id is the new primary key. person_id and country are no longer primary keys

class Nationality(Model):
    id = Column(Integer, primary_key=True, index=True)
    person_id = Column(Integer,
                        ForeignKey('person.id',
                                   ondelete='CASCADE'),
                        index=True)

    person = relationship(
        'Person',
        backref=backref(
            'nationalities',
            cascade='all, delete-orphan'
        )
    )

    country = Column(CountryType, index=True)

[TrilceAC]

Ordering by code makes sense. You can use total_ordering decorator. PRs welcome

Happy to do. Just a question: Should Country be comparable to any str, or just Country to Country?

    def __lt__(self, other):
        if isinstance(other, Country):
            return self.code < other.code
        if isinstance(other, str): # <----------------- Keep or remove this case?
            return self.code < other
        return NotImplemented