Mutable column from mixin does not track mutation

While trying to diagnose an issue I had with social-app-flask-sqlalchemy, I found a somewhat unintuitive behavior with sqlalchemy where I'm not sure if it's expected behavior or a bug.

Consider the following snippet:

from sqlalchemy import create_engine, Column, Integer, PickleType
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import sessionmaker

Base = declarative_base()

class A(Base):
    __abstract__ = True

class B(Base):
    id = Column(Integer, primary_key=True)
    __tablename__ = 'some_table'
    my_data = Column(MutableDict.as_mutable(PickleType))

class C(A, B):
    pass

engine = create_engine('sqlite://')
session_factory = sessionmaker(bind=engine)
db_session = session_factory()

Base.metadata.create_all(engine)

assert B.my_data.type.__class__ is PickleType

c_instance = C(my_data={'foo': 'bar'})
db_session.add(c_instance)
db_session.commit()
loaded_instance = db_session.query(C).first()
loaded_instance.my_data.update(baz=1)

assert loaded_instance.my_data['baz'] == 1

assert loaded_instance in db_session.dirty

This runs without issues. Now if we change the superclass of B to object, the last assertion errors out. Up until there, everything works fine though.

It turns out that by not having B subclass the declarative_base directly, the type of my_data is no longer enforced to be MutableDict, but whatever type we give when instantiating an object (a dict in this case). Obviously, this also means that changes to my_data are no longer tracked. However, my_data still uses PickleType as its data type, so the effect is not immediately visible.

I originally stumbled upon this when changes to extra_data in social-app-flask-sqlalchemy were not written to the DB. social-app-flask-sqlalchemy uses an ORM that is similar to the one shown above—the SQLAlchemyUserMixin class which holds the extra_data column is not subclassing declarative_base, but its subclass UserSocialAuth does via _AppSession.

Now I'm not sure where to report it as an issue, with sqlalchemy or social-app-flask-sqlalchemy. Any thoughts?

Solution

You should declare complex mixin columns such as yours using the declared_attr decorator. Simple column declarations will be copied from the mixin:

To accomplish this, the declarative extension creates a copy of each Column object encountered on a class that is detected as a mixin.

and apparently this does not play well with mutation tracking. The documentation is perhaps a bit vague on what constructs should use the decorator.

This copy mechanism is limited to simple columns that have no foreign keys, as a ForeignKey itself contains references to columns which can’t be properly recreated at this level. For columns that have foreign keys, as well as for the variety of mapper-level constructs that require destination-explicit context, the declared_attr decorator is provided so that patterns common to many classes can be defined as callables

Emphasis added, though I might've misunderstood what "destination-explicit context" means in this context. So do this:

class B(object):
    ...
    @declared_attr
    def my_data(cls):
        return Column(MutableDict.as_mutable(PickleType))

instead, if B is to be a mixin class. An example of both the failing simple declaration and declared_attr:

In [2]: from sqlalchemy.ext.mutable import MutableDict

In [3]: class MixinA:
   ...:     extra = Column(MutableDict.as_mutable(PickleType))
   ...:

In [4]: from sqlalchemy.ext.declarative import declared_attr

In [5]: class MixinB:
   ...:     @declared_attr
   ...:     def extra(cls):
   ...:         return Column(MutableDict.as_mutable(PickleType))
   ...:

In [6]: class A(MixinA, Base):
   ...:     __tablename__ = 'a'
   ...:     id = Column(Integer, primary_key=True, autoincrement=True)
   ...:

In [7]: class B(MixinB, Base):
   ...:     __tablename__ = 'b'
   ...:     id = Column(Integer, primary_key=True, autoincrement=True)
   ...:

In [8]: metadata.create_all()

and in action:

In [9]: a = A(extra={})

In [10]: b = B(extra={})

In [11]: session.add(a)

In [12]: session.add(b)

In [13]: session.commit()

In [14]: session.query(A.extra).first()
Out[14]: ({})

In [15]: session.query(B.extra).first()
Out[15]: ({})

In [16]: b.extra['b'] = 1

In [17]: session.commit()

In [18]: session.query(B.extra).first()
Out[18]: ({'b': 1})

In [19]: a.extra['a'] = 1

In [20]: session.commit()

In [21]: session.query(A.extra).first()
Out[21]: ({})

In [22]: b.extra['bb'] = 2

In [23]: assert b in session.dirty