Mixin of custom TypeEngine with ColumnProperty doesn't work

[Former user]

I'm using GeoAlchemy which defines a custom type derived from TypeEngine and it never makes it into table columns, and though ignored during the creation phase. Below is a test case and I'm attaching a patch.

class LocalizedThingyMixin(object): title = Column('title', types.String(128), nullable=False) description = Column('description', types.Text, nullable=False) location = GeometryColumn('location', Point(2), comparator=PGComparator)

class Event(Base,LocalizedThingyMixin): """docstring for Event""" tablename = "event" id = Column(types.Integer, Sequence('object_sequence'), primary_key=True)

location is never created, because it is ignored during the mixing recursion phase.

[Former user]

patch

[Former user]

Sorry, didn't enclose test case in code block...

class LocalizedThingyMixin(object):
    title = Column('title', types.String(128), nullable=False)
    description = Column('description', types.Text, nullable=False)
    location = GeometryColumn('location', Point(2), comparator=PGComparator)

class Event(Base,LocalizedThingyMixin):
    """docstring for Event"""
    __tablename__ = "event"
    id = Column(types.Integer, Sequence('object_sequence'), primary_key=True)

Ilya Sterin

[Mike Bayer]

Here's why the patch doesn't work. At the very least, the column_property would have to copy its contained columns. But that still doesn't prevent the scenario below and actually makes it worse:

        class MyMixin(object):
            prop = Column('prop', String(50))
            prop_hoho = column_property(prop + 'hoho')

        class MyModel(Base, MyMixin):
            __tablename__='test'
            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)

        class MyOtherModel(Base, MyMixin):
            __tablename__='othertest'
            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)

        compile_mappers()

        assert MyModel.prop.property.columns[0](0) is MyModel.__table__.c.prop
        # fails
        assert MyModel.prop_hoho.property.columns[0](0) is MyModel.__table__.c.prop

        assert MyOtherModel.prop.property.columns[0](0) is MyOtherModel.__table__.c.prop

        # fails
        assert MyOtherModel.prop_hoho.property.columns[0](0) is MyOtherModel.__table__.c.prop

the above pattern "works" for simple cases but is sloppy and probably begins to have subtle issues since the prop_hoho is not referencing the correct Column object.

[Mike Bayer]

even worse:

        class MyMixin(object):
            prop_hoho = column_property(Column('prop', String(50)))

        class MyModel(Base, MyMixin):
            __tablename__='test'
            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)

        class MyOtherModel(Base, MyMixin):
            __tablename__='othertest'
            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)

        assert MyModel.__table__.c.prop is not None
        assert MyOtherModel.__table__.c.prop is not None

"prop" is only getting placed on one of the tables. no errors are raised and the mapping is pretty severely broken.

[Former user]

Yeah, sorry, I'm not familiar with SQLAlchemy internals. I just started using it a week ago and ran into this issue. Fix seemed naively easy and rightfully so.

I can dig in deeper, but it'll take a while to get familiar with internals, might be faster for someone with the knowledge to fix it.

Let me know if I can do anything.

[Mike Bayer]

a new proposed patch is added. This isolates the point at which we'd have to copy the expression. still doesn't work though...

    def test_column_property_conflict_two(self):
        # We cannot copy "prop_hoho"
        # and still have it reference the correct 'prop' column.
        class MyMixin(object):
            prop = Column('prop', String(50))
            prop_hoho = column_property(prop)

        class MyModel(Base, MyMixin):
            __tablename__='test'
            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)

        class MyOtherModel(Base, MyMixin):
            __tablename__='othertest'
            id = Column(Integer, primary_key=True, test_needs_autoincrement=True)

        compile_mappers()
        assert MyModel.prop_hoho.property.columns[0](0) is MyModel.__table__.c.prop
        # boom
        assert MyModel.prop.property.columns[0](0) is MyModel.__table__.c.prop

[Mike Bayer]

ill keep it on 0.6.1 for now, so i can take another look before i put out 0.6.1 in a few weeks, but I really don't have time to play with it more today.

[Former user]

Thanks a lot for the fix.

I don't see it in tip yet. Did you commit this to some other branch?

[Mike Bayer]

the fix does not work. there is no solution for this issue as of yet.

I am considering something along the lines of:

class MyMixin(object):
    prop = Column(Integer)

    @classproperty
    def prop_foo(cls):
        return sa.orm.column_property(cls.prop)

the basic fact is that column_property() references other aspects of the table metadata by value. Copying it is not that simple, as is also the case with columns with foreign keys, etc. Expect to see many more mixin-configurations that don't work as expected without many more hurdles being overcome, as mixins were never part of declarative's original idea.

[Mike Bayer]

55ae7d784212c3b03ba74d27e1e088486054c993

[Mike Bayer]

Removing milestone: 0.6.2 (automated comment)