Thread: [Sqlalchemy-tickets] [sqlalchemy] #1751: foreign keys on mixins are broken
Brought to you by:
zzzeek
From: sqlalchemy <mi...@zz...> - 2010-03-26 22:49:55
|
#1751: foreign keys on mixins are broken --------------------+------------------------------------------------------- Reporter: guest | Owner: paj Type: defect | Status: new Priority: medium | Milestone: Component: access | Severity: no triage selected yet Keywords: | Status_field: awaiting triage --------------------+------------------------------------------------------- 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](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. -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-03-27 03:01:21
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself --------------------+------------------------------------------------------- Reporter: guest | Owner: zzzeek Type: defect | Status: new Priority: medium | Milestone: 0.6.xx Component: ext | Severity: minor - half an hour Keywords: | Status_field: in queue --------------------+------------------------------------------------------- Changes (by zzzeek): * severity: no triage selected yet => minor - half an hour * component: access => ext * milestone: => 0.6.xx * owner: paj => zzzeek * status_field: awaiting triage => in queue Comment: 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. -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:1> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-03-27 03:16:02
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ---------------------------------+------------------------------------------ Reporter: guest | Owner: zzzeek Type: defect | Status: closed Priority: medium | Milestone: 0.6.0 Component: ext | Severity: minor - half an hour Resolution: fixed | Keywords: Status_field: completed/closed | ---------------------------------+------------------------------------------ Changes (by zzzeek): * status: new => closed * status_field: in queue => completed/closed * resolution: => fixed * milestone: 0.6.xx => 0.6.0 Comment: 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 ra848720aab42. -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:2> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-03-27 06:22:35
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ---------------------------------+------------------------------------------ Reporter: guest | Owner: zzzeek Type: defect | Status: closed Priority: medium | Milestone: 0.6.0 Component: ext | Severity: minor - half an hour Resolution: fixed | Keywords: Status_field: completed/closed | ---------------------------------+------------------------------------------ Comment(by guest): Replying to [comment:1 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. -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:3> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-03-28 23:08:37
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ---------------------------------+------------------------------------------ Reporter: guest | Owner: zzzeek Type: defect | Status: closed Priority: medium | Milestone: 0.6.0 Component: ext | Severity: minor - half an hour Resolution: fixed | Keywords: Status_field: completed/closed | ---------------------------------+------------------------------------------ Comment(by guest): [Different 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 -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:4> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-03-28 23:25:59
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ---------------------------------+------------------------------------------ Reporter: guest | Owner: zzzeek Type: defect | Status: closed Priority: medium | Milestone: 0.6.0 Component: ext | Severity: minor - half an hour Resolution: fixed | Keywords: Status_field: completed/closed | ---------------------------------+------------------------------------------ Comment(by guest): Googling around a bit, I found a discussion [http://groups.google.ru/group/sqlalchemy/browse_thread/thread/99812e0ca1f8cc7c 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. -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:5> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-03-29 00:07:48
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ---------------------------------+------------------------------------------ Reporter: guest | Owner: zzzeek Type: defect | Status: closed Priority: medium | Milestone: 0.6.0 Component: ext | Severity: minor - half an hour Resolution: fixed | Keywords: Status_field: completed/closed | ---------------------------------+------------------------------------------ Comment(by zzzeek): Replying to [comment:4 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 -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:6> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-07-03 18:12:15
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ----------------------------+----------------------------------------------- Reporter: guest | Owner: zzzeek Type: defect | Status: reopened Priority: medium | Milestone: 0.6.2 Component: ext | Severity: minor - half an hour Resolution: | Keywords: Status_field: in progress | ----------------------------+----------------------------------------------- Changes (by zzzeek): * status: closed => reopened * status_field: completed/closed => in progress * resolution: fixed => * milestone: 0.6.0 => 0.6.2 Comment: a solution for this issue, as well as #1796 and #1805 is in progress. -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:7> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |
From: sqlalchemy <mi...@zz...> - 2010-07-03 18:54:21
|
#1751: tighten contraints on whats allowed in mixins - including wihtin the column itself ---------------------------------+------------------------------------------ Reporter: guest | Owner: zzzeek Type: defect | Status: closed Priority: medium | Milestone: 0.6.2 Component: ext | Severity: minor - half an hour Resolution: fixed | Keywords: Status_field: completed/closed | ---------------------------------+------------------------------------------ Changes (by zzzeek): * status: reopened => closed * resolution: => fixed * status_field: in progress => completed/closed Comment: r02a8754aff2b -- Ticket URL: <http://www.sqlalchemy.org/trac/ticket/1751#comment:8> sqlalchemy <http://www.sqlalchemy.org/> The Database Toolkit for Python |