On Jun 4, 2010, at 3:21 PM, Gunnlaugur Briem wrote:
> Hi,
>
> if I declaratively map a class with a mix-in and with a __table__
> definition that contains the columns in the mix-in, like so:
>
> class MyMixin(object):
> id = Column(Integer, primary_key=True)
> def foo(self):
> return 'bar'+str(self.id)
>
> class MyModel(Base,MyMixin):
> __table__ = Table('test', Base.metadata,
> Column('id', Integer, primary_key=True),
> Column('name', String(1000), nullable=False, index=True)
> )
>
> … then I get a fairly cryptic error:
>
> Traceback (most recent call last):
> File "mixin_trouble.py", line 12, in <module>
> class MyModel(Base,MyMixin):
> File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/ext/
> declarative.py", line 830, in __init__
> _as_declarative(cls, classname, cls.__dict__)
> File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/ext/
> declarative.py", line 681, in _as_declarative
> if tablename or k not in parent_columns:
> TypeError: Error when calling the metaclass bases
> argument of type 'NoneType' is not iterable
>
> The real reason is that the case of __table__ with a mix-in is not
> handled. If MyModel were defined with __tablename__ and id =
> Column(Integer, primary_key=True), then tablename would be truey and
> parent_columns would not be checked, so this would not come up.
>
> We could just improve the error message by checking for this
> unsupported case. But it seems fairly easy to support it. This simple
> change makes the above case work, and breaks no unit tests:
>
> diff -r dd463bfb847d lib/sqlalchemy/ext/declarative.py
> --- a/lib/sqlalchemy/ext/declarative.py Mon May 31 15:22:55 2010 -0400
> +++ b/lib/sqlalchemy/ext/declarative.py Fri Jun 04 19:05:16 2010 +0000
> @@ -670,7 +670,9 @@
> "Columns with foreign keys to other
> columns "
> "are not allowed on declarative
> mixins at this time."
> )
> - if name not in dict_:
> + if name not in dict_ and not (
> + '__table__' in dict_ and name in
> dict_['__table__'].c
> + ):
>
> potential_columns[name]=column_copies[obj]=obj.copy()
> elif isinstance(obj, RelationshipProperty):
> raise exceptions.InvalidRequestError(
>
> I may be missing something else that needs to be done in this case,
> but in any case this gets my codebase (which I've just modified to use
> a mix-in like this instead of a metaclass) running and passing its
> tests without problems.
as long as all tests pass it is fine. If you could give me a patch that
includes a test for this in test_declarative, that would be supremely helpful
(if you want to make a trac ticket and target it at the 0.6.2 milestone).
>
> Also, since I've got your attention there, shouldn't this place really
> check (in both this new __table__ case and the already-supported
> __tablename__ case) that the column definitions are identical, or at
> least mostly equivalent? If they don't, then the mix-in class'
> expectations are not fulfilled and that may cause all sorts of trouble
> so it's better to bump into it right away.
do you mean, if the new class overrides what the mixin provides ? i'm not
sure why we'd need to do anything if the class overrides the mixin.
>
> To that end, is there an existing way to check column definition
> equality/equivalence, i.e. comparing at least type, constraints and
> default and primary_key, and maybe also autoincrement, base_columns,
> etc.?
theres some of that in the unit tests but in general I try not to make such
functionality a requirement of anything, as it's difficult to maintain and also
has unresolved edge cases in the case of custom subclasses, custom types,
dialect-specific types, potential dialect-specific flags, etc.
--
You received this message because you are subscribed to the Google Groups
"sqlalchemy" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sqlalchemy?hl=en.
