tighten contraints on whats allowed in mixins - including wihtin the column itself

Issue #1751 resolved
Former user created an issue

While playing around with declerative mixins I probably found another issue.

Using this script (already stripped down, removed all magic)

from datetime import datetime
from sqlalchemy.util import classproperty
from sqlalchemy import Table, Column, ForeignKey, Integer, String, DateTime, \
    create_engine
from sqlalchemy.orm import sessionmaker, backref, relationship
from sqlalchemy.ext.declarative import declarative_base


engine = create_engine('sqlite://', echo=True)

Base = declarative_base(bind=engine)
session = sessionmaker()


class User(Base):
    __tablename__ = 'users'
    id = Column(Integer, primary_key=True)


class ModelHistoryMixin(object):
    id = Column(Integer, primary_key=True)
    user = Column(Integer, ForeignKey(User.id))


class Article(Base):
    __tablename__ = 'articles'
    id = Column(Integer, primary_key=True)


class ArticleHistory(Base, ModelHistoryMixin):
    __tablename__ = 'articles_history'

Base.metadata.create_all()

I get this traceback:

Traceback (most recent call last):
  File "test.py", line 36, in <module>
    Base.metadata.create_all()
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/schema.py", line 1945, in create_all
    bind.create(self, checkfirst=checkfirst, tables=tables)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 1466, in create
    self._run_visitor(ddl.SchemaGenerator, entity, connection=connection, **kwargs)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 1497, in _run_visitor
    visitorcallable(self.dialect, conn, **kwargs).traverse(element)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/visitors.py", line 86, in traverse
    return traverse(obj, self.__traverse_options__, self._visitor_dict)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/visitors.py", line 197, in traverse
    return traverse_using(iterate(obj, opts), obj, visitors)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/visitors.py", line 191, in traverse_using
    meth(target)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/ddl.py", line 42, in visit_metadata
    self.traverse_single(table)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/visitors.py", line 76, in traverse_single
    return meth(obj)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/ddl.py", line 55, in visit_table
    self.connection.execute(schema.CreateTable(table))
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 1086, in execute
    return Connection.executors[c](c)(self, object, multiparams, params)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 1133, in _execute_ddl
    compiled_ddl=ddl.compile(dialect=self.dialect),
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/expression.py", line 1257, in compile
    compiler.compile()
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 663, in compile
    self.string = self.process(self.statement)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 676, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/visitors.py", line 47, in _compiler_dispatch
    return getter(visitor)(self, **kw)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1095, in visit_create_table
    const = self.create_table_constraints(table)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1113, in create_table_constraints
    (self.process(constraint) for constraint in constraints 
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1112, in <genexpr>
    return ", \n\t".join(p for p in
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1119, in <genexpr>
    not getattr(constraint, 'use_alter', False)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/engine/base.py", line 676, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/visitors.py", line 47, in _compiler_dispatch
    return getter(visitor)(self, **kw)
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1241, in visit_foreign_key_constraint
    for f in constraint._elements.values()),
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1241, in <genexpr>
    for f in constraint._elements.values()),
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1481, in quote
    if self._requires_quotes(ident):
  File "/home/ente/Projects/inyoka/lib/python2.6/site-packages/sqlalchemy/sql/compiler.py", line 1462, in _requires_quotes
    lc_value = value.lower()
AttributeError: 'NoneType' object has no attribute 'lower'

If you remove the user column from the mixin everything works quite fine. Would be very cool if you could find a solution for that too.

Thanks very much.

Regards, Christopher.

Comments (9)

  1. Mike Bayer repo owner

    this is exactly why i didnt want declarative to support mixins, this is very close to abuse of the feature. "User.id" is not the column object that's going to be used here, its going to get copied, and you really want the foreign key here to be "Article.id" If i were to fix this issue, I'd make it raise an error on the fact that you used a ForeignKey on a mixin column.

  2. Mike Bayer repo owner

    sorry, it was exactly my fear that people would immediately think they could put all kinds of relationships to other objects inside of mixins and that sqlalchemy would magically "figure out" how it should all connect together. but it can't, and its way out of scope to spend weeks figuring out rules to make it all "make sense" some how. foreign keys and relationships on mixins are explicitly disallowed in b7a2d7de4854c26ea7773b1002852d4245cfcc10.

  3. Former user Account Deleted

    Replying to zzzeek:

    this is exactly why i didnt want declarative to support mixins, this is very close to abuse of the feature. "User.id" is not the column object that's going to be used here, its going to get copied, and you really want the foreign key here to be "Article.id" If i were to fix this issue, I'd make it raise an error on the fact that you used a ForeignKey on a mixin column.

    Well, it's not Article.id I want to use here. This column should represent the user that created this revision.

    But I guess for those things there is all that polymorphic stuff. This should work better than mixins. Thank you anyway!

    Regards, Christopher.

  4. Former user Account Deleted

    user here...

    While I can understand that it is hard to support this in SQLAlchemy (I am as of yet unable to follow all the magic that SA is doing), it is a severe limitation IMHO.

    There are a lot of nice things in python to support the DRY principle (inheritance, decorators, metaclasses, etc.) which are broken by this.

    I also wanted to do something like what Christopher wanted to do and ran into the same problem. It is nice to get a good error message now. I'd rather have an alternative to do this. No need to support doing stuff like this in mixin classes. It would suffices to have some decorator pattern for this.

    What I am asking for is a hint what would be the shortest way to implement common columns over multiple tables where there is no matching inheritance hierarchy. Think comment fields for all kinds of data. Any hints?

    Kudos, Torsten Landschoff

  5. Former user Account Deleted

    Googling around a bit, I found a discussion noting that decorators can be used for this. I was mostly successful with this approach, but unfortunately the __mapper_args__ can not be set in the class decorator as it seems!? The reason is still unclear to me but that's a small issue I can live with for now.

  6. Mike Bayer repo owner

    Replying to guest:

    I also wanted to do something like what Christopher wanted to do and ran into the same problem. It is nice to get a good error message now. I'd rather have an alternative to do this. No need to support doing stuff like this in mixin classes. It would suffices to have some decorator pattern for this.

    What I am asking for is a hint what would be the shortest way to implement common columns over multiple tables where there is no matching inheritance hierarchy. Think comment fields for all kinds of data. Any hints?

    using custom metaclasses allows any sort of on-class-create activity to occur. the mixin pattern has been kindly donated to us by Chris Withers, and could be extended to support many as-yet not implemented use cases - think defining a Python descriptor that represents a column.

    The error message in question here only applies to areas of functionality that have not yet been implemented, namely defining columns and relations that reference other tables/classes.

    As very specific use cases are identified, please add new tickets to trac as feature enhancements. I've added a new component to trac called "declarative" with Chris Withers as its owner. Declarative is complete for my current needs so its up to the community to define and contribute new features they'd like to see added (including full unit tests).

    Kudos, Torsten Landschoff

  7. Log in to comment