diff options
| -rw-r--r-- | CHANGES | 8 | ||||
| -rwxr-xr-x | lib/sqlalchemy/ext/declarative.py | 25 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 4 | ||||
| -rw-r--r-- | test/ext/test_declarative.py | 89 |
4 files changed, 114 insertions, 12 deletions
@@ -149,10 +149,10 @@ CHANGES not just an attribute technique. [ticket:1915] - Fixed bug whereby columns on a mixin wouldn't propagate - correctly to a single-table inheritance scheme where - the attribute name is different than that of the column. - [ticket:1930]. Note [ticket:1931] which is the same - issue for joined inh, not yet resolved. + correctly to a single-table, or joined-table, + inheritance scheme where the attribute name is + different than that of the column. [ticket:1930], + [ticket:1931]. - engine diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py index 211b6724a..fabd9aaf9 100755 --- a/lib/sqlalchemy/ext/declarative.py +++ b/lib/sqlalchemy/ext/declarative.py @@ -855,7 +855,7 @@ from sqlalchemy.orm.interfaces import MapperProperty from sqlalchemy.orm.properties import RelationshipProperty, ColumnProperty from sqlalchemy.orm.util import _is_mapped_class from sqlalchemy import util, exceptions -from sqlalchemy.sql import util as sql_util +from sqlalchemy.sql import util as sql_util, expression __all__ = 'declarative_base', 'synonym_for', \ @@ -938,9 +938,6 @@ def _as_declarative(cls, classname, dict_): "on declarative mixin classes. ") if name not in dict_ and not ( '__table__' in dict_ and - # TODO: coverage here when obj.name is not - # None and obj.name != name and obj.name in - # table.c (obj.name or name) in dict_['__table__'].c ): potential_columns[name] = \ @@ -1119,7 +1116,25 @@ def _as_declarative(cls, classname, dict_): set([c.key for c in inherited_table.c if c not in inherited_mapper._columntoproperty]) exclude_properties.difference_update([c.key for c in cols]) - + + # look through columns in the current mapper that + # are keyed to a propname different than the colname + # (if names were the same, we'd have popped it out above, + # in which case the mapper makes this combination). + # See if the superclass has a similar column property. + # If so, join them together. + for k, col in our_stuff.items(): + if not isinstance(col, expression.ColumnElement): + continue + if k in inherited_mapper._props: + p = inherited_mapper._props[k] + if isinstance(p, ColumnProperty): + # note here we place the superclass column + # first. this corresponds to the + # append() in mapper._configure_property(). + # change this ordering when we do [ticket:1892] + our_stuff[k] = p.columns + [col] + cls.__mapper__ = mapper_cls(cls, table, properties=our_stuff, diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index d27b99601..378570723 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -639,7 +639,9 @@ class Mapper(object): "or more attributes for these same-named columns " "explicitly." % (prop.columns[-1], column, key)) - + + # this hypothetically changes to + # prop.columns.insert(0, column) when we do [ticket:1892] prop.columns.append(column) self._log("appending to existing ColumnProperty %s" % (key)) diff --git a/test/ext/test_declarative.py b/test/ext/test_declarative.py index 1b3e72fc7..0202aa69f 100644 --- a/test/ext/test_declarative.py +++ b/test/ext/test_declarative.py @@ -2305,6 +2305,13 @@ class DeclarativeMixinTest(DeclarativeTestBase): eq_(General.__table__.kwargs, {'mysql_engine': 'InnoDB'}) def test_columns_single_table_inheritance(self): + """Test a column on a mixin with an alternate attribute name, + mapped to a superclass and single-table inheritance subclass. + The superclass table gets the column, the subclass shares + the MapperProperty. + + """ + class MyMixin(object): foo = Column('foo', Integer) bar = Column('bar_newname', Integer) @@ -2318,8 +2325,18 @@ class DeclarativeMixinTest(DeclarativeTestBase): class Specific(General): __mapper_args__ = {'polymorphic_identity': 'specific'} - @testing.fails_if(lambda: True, "Unhandled declarative use case") + assert General.bar.prop.columns[0] is General.__table__.c.bar_newname + assert len(General.bar.prop.columns) == 1 + assert Specific.bar.prop is General.bar.prop + def test_columns_joined_table_inheritance(self): + """Test a column on a mixin with an alternate attribute name, + mapped to a superclass and joined-table inheritance subclass. + Both tables get the column, in the case of the subclass the two + columns are joined under one MapperProperty. + + """ + class MyMixin(object): foo = Column('foo', Integer) bar = Column('bar_newname', Integer) @@ -2334,6 +2351,52 @@ class DeclarativeMixinTest(DeclarativeTestBase): __tablename__ = 'sub' id = Column(Integer, ForeignKey('test.id'), primary_key=True) __mapper_args__ = {'polymorphic_identity': 'specific'} + + assert General.bar.prop.columns[0] is General.__table__.c.bar_newname + assert len(General.bar.prop.columns) == 1 + assert Specific.bar.prop is not General.bar.prop + assert len(Specific.bar.prop.columns) == 2 + assert Specific.bar.prop.columns[0] is General.__table__.c.bar_newname + assert Specific.bar.prop.columns[1] is Specific.__table__.c.bar_newname + + def test_column_join_checks_superclass_type(self): + """Test that the logic which joins subclass props to those + of the superclass checks that the superclass property is a column. + + """ + class General(Base): + __tablename__ = 'test' + id = Column(Integer, primary_key=True) + general_id = Column(Integer, ForeignKey('test.id')) + type_ = relationship("General") + + class Specific(General): + __tablename__ = 'sub' + id = Column(Integer, ForeignKey('test.id'), primary_key=True) + type_ = Column('foob', String(50)) + + assert isinstance(General.type_.property, sa.orm.RelationshipProperty) + assert Specific.type_.property.columns[0] is Specific.__table__.c.foob + + def test_column_join_checks_subclass_type(self): + """Test that the logic which joins subclass props to those + of the superclass checks that the subclass property is a column. + + """ + def go(): + class General(Base): + __tablename__ = 'test' + id = Column(Integer, primary_key=True) + type_ = Column('foob', Integer) + + class Specific(General): + __tablename__ = 'sub' + id = Column(Integer, ForeignKey('test.id'), primary_key=True) + specific_id = Column(Integer, ForeignKey('sub.id')) + type_ = relationship("Specific") + assert_raises_message( + sa.exc.ArgumentError, "column 'foob' conflicts with property", go + ) def test_table_args_overridden(self): @@ -2777,7 +2840,6 @@ class DeclarativeMixinTest(DeclarativeTestBase): class ColumnMixin: tada = Column(Integer) - def go(): class Model(Base, ColumnMixin): @@ -2791,6 +2853,29 @@ class DeclarativeMixinTest(DeclarativeTestBase): "Can't add additional column 'tada' when " "specifying __table__", go) + def test_table_in_model_and_different_named_alt_key_column_in_mixin(self): + + # here, the __table__ has a column 'tada'. We disallow + # the add of the 'foobar' column, even though it's + # keyed to 'tada'. + + class ColumnMixin: + tada = Column('foobar', Integer) + + def go(): + + class Model(Base, ColumnMixin): + + __table__ = Table('foo', Base.metadata, + Column('data',Integer), + Column('tada', Integer), + Column('id', Integer,primary_key=True)) + foo = relationship("Dest") + + assert_raises_message(sa.exc.ArgumentError, + "Can't add additional column 'foobar' when " + "specifying __table__", go) + def test_table_in_model_overrides_different_typed_column_in_mixin(self): class ColumnMixin: |