Setting unexpected type for relationship throws exception but modifies session state

Issue #3790 wontfix
Konsta Vesterinen created an issue

Consider the following model definition and data.

import sqlalchemy as sa
from sqlalchemy.ext.declarative import declarative_base, synonym_for

Base = declarative_base()


class Section(Base):
    __tablename__ = 'section'
    id = sa.Column(sa.Integer, primary_key=True)
    name = sa.Column(sa.String)


class Question(Base):
    __tablename__ = 'question'
    id = sa.Column(sa.Integer, primary_key=True)
    name = sa.Column(sa.String)
    section_id = sa.Column(sa.Integer, sa.ForeignKey(Section.id))
    section = sa.orm.relationship(Section, backref='questions')


engine = sa.create_engine('sqlite:///:memory:')
conn = engine.connect()

sa.orm.configure_mappers()
Base.metadata.create_all(conn)

Session = sa.orm.sessionmaker(bind=conn)
session = Session()

section = Section(questions=[Question(name='1'), Question(name='2')])
session.add(section)
session.commit()

Now if we did something like this accidentally

section.questions[0] = 'Some updated question name'

It throws Exception as expected but it also modifies session state so that calling

session.commit()

will result our given section having only one question. Maybe it would be better if set operations that threw an error would not affect the session state.

I encountered this issue while doing stuff in console and later find out that I accidentally deleted one record.

Comments (4)

  1. Mike Bayer repo owner

    im not seeing any way how. Here's what "some_collection[index] = obj" looks like:

            def __setitem__(self, index, value):
                if not isinstance(index, slice):
                    existing = self[index]
                    if existing is not None:
                        __del(self, existing)
                    value = __set(self, value)
                    fn(self, index, value)
    

    the __set fails and the list is never mutated. But you have a backref on there. Here's what that looks like in pdb:

    (Pdb) from sqlalchemy import inspect
    (Pdb) inspect(section.questions[0]).attrs.section.history
    History(added=[None], unchanged=(), deleted=[<__main__.Section object at 0x7f0465447b90>])
    

    that is, the operation has already done the deletion events which affects the backref, and the Question here thinks its Section has been removed from it. Relationship mutations involve several steps. In this example, only the last one is failing. The previous ones are not trivially reversible.

  2. Mike Bayer repo owner

    well, OK, simple things like "check for the mapping" can happen, though this still involves deep changes in order to avoid having to pull the instance_state more than once. But this solution wouldn't be generalizable. If someone's set event throws an error for some othe reason, it still won't reverse the delete event.

  3. Konsta Vesterinen reporter

    Ok thanks for the explanation and yeah indeed it seems this is a very hard problem to fix - not worth the effort I would say.

  4. Mike Bayer repo owner

    session failure cases are very hard to clean up for in the general case and this would be difficult to maintain

  5. Log in to comment