- [bug] Fixed a disconnect that slowly evolved

between a @declared_attr Column and a
directly-defined Column on a mixin. In both
cases, the Column will be applied to the
declared class' table, but not to that of a
joined inheritance subclass.   Previously,
the directly-defined Column would be placed
on both the base and the sub table, which isn't
typically what's desired.  [ticket:2565]

3 files changed, 81 insertions, 8 deletions

diff --git a/CHANGES b/CHANGES
index 860057db2..00b60d510 100644
--- a/CHANGES
+++ b/CHANGES
@@ -252,6 +252,16 @@ underneath "0.7.xx".
     including any user-defined value as well
     as association proxy objects.  [ticket:2517]
 
+  - [bug] Fixed a disconnect that slowly evolved
+    between a @declared_attr Column and a
+    directly-defined Column on a mixin. In both
+    cases, the Column will be applied to the
+    declared class' table, but not to that of a
+    joined inheritance subclass.   Previously,
+    the directly-defined Column would be placed
+    on both the base and the sub table, which isn't
+    typically what's desired.  [ticket:2565]
+
   - [feature] *Very limited* support for
     inheriting mappers to be GC'ed when the
     class itself is deferenced.  The mapper
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 40c8c6ef6..8e8f5626c 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -98,6 +98,11 @@ def _as_declarative(cls, classname, dict_):
             elif base is not cls:
                 # we're a mixin.
                 if isinstance(obj, Column):
+                    if getattr(cls, name) is not obj:
+                        # if column has been overridden
+                        # (like by the InstrumentedAttribute of the
+                        # superclass), skip
+                        continue
                     if obj.foreign_keys:
                         raise exc.InvalidRequestError(
                         "Columns with foreign keys to other columns "
@@ -127,8 +132,7 @@ def _as_declarative(cls, classname, dict_):
 
     # apply inherited columns as we should
     for k, v in potential_columns.items():
-        if tablename or (v.name or k) not in parent_columns:
-            dict_[k] = v
+        dict_[k] = v
 
     if inherited_table_args and not tablename:
         table_args = None
diff --git a/test/ext/declarative/test_mixin.py b/test/ext/declarative/test_mixin.py
index 6c2233302..bec9a659d 100644
--- a/test/ext/declarative/test_mixin.py
+++ b/test/ext/declarative/test_mixin.py
@@ -369,10 +369,9 @@ class DeclarativeMixinTest(DeclarativeTestBase):
 
         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 Specific.__table__.c.bar_newname
-        assert Specific.bar.prop.columns[1] is General.__table__.c.bar_newname
+        assert Specific.bar.prop is General.bar.prop
+        eq_(len(Specific.bar.prop.columns), 1)
+        assert Specific.bar.prop.columns[0] is General.__table__.c.bar_newname
 
     def test_column_join_checks_superclass_type(self):
         """Test that the logic which joins subclass props to those
@@ -717,7 +716,7 @@ class DeclarativeMixinTest(DeclarativeTestBase):
         eq_(Specific.__table__.name, 'specific')
         eq_(Generic.__table__.c.keys(), ['timestamp', 'id',
             'python_type'])
-        eq_(Specific.__table__.c.keys(), ['timestamp', 'id'])
+        eq_(Specific.__table__.c.keys(), ['id'])
         eq_(Generic.__table__.kwargs, {'mysql_engine': 'InnoDB'})
         eq_(Specific.__table__.kwargs, {'mysql_engine': 'InnoDB'})
 
@@ -755,9 +754,69 @@ class DeclarativeMixinTest(DeclarativeTestBase):
         eq_(BaseType.__table__.kwargs, {'mysql_engine': 'InnoDB'})
         assert Single.__table__ is BaseType.__table__
         eq_(Joined.__table__.name, 'joined')
-        eq_(Joined.__table__.c.keys(), ['timestamp', 'id'])
+        eq_(Joined.__table__.c.keys(), ['id'])
         eq_(Joined.__table__.kwargs, {'mysql_engine': 'InnoDB'})
 
+    def test_col_copy_vs_declared_attr_joined_propagation(self):
+        class Mixin(object):
+            a = Column(Integer)
+
+            @declared_attr
+            def b(cls):
+                return Column(Integer)
+
+        class A(Mixin, Base):
+            __tablename__ = 'a'
+            id = Column(Integer, primary_key=True)
+
+        class B(A):
+            __tablename__ = 'b'
+            id = Column(Integer, ForeignKey('a.id'), primary_key=True)
+
+        assert 'a' in A.__table__.c
+        assert 'b' in A.__table__.c
+        assert 'a' not in B.__table__.c
+        assert 'b' not in B.__table__.c
+
+    def test_col_copy_vs_declared_attr_joined_propagation_newname(self):
+        class Mixin(object):
+            a = Column('a1', Integer)
+
+            @declared_attr
+            def b(cls):
+                return Column('b1', Integer)
+
+        class A(Mixin, Base):
+            __tablename__ = 'a'
+            id = Column(Integer, primary_key=True)
+
+        class B(A):
+            __tablename__ = 'b'
+            id = Column(Integer, ForeignKey('a.id'), primary_key=True)
+
+        assert 'a1' in A.__table__.c
+        assert 'b1' in A.__table__.c
+        assert 'a1' not in B.__table__.c
+        assert 'b1' not in B.__table__.c
+
+    def test_col_copy_vs_declared_attr_single_propagation(self):
+        class Mixin(object):
+            a = Column(Integer)
+
+            @declared_attr
+            def b(cls):
+                return Column(Integer)
+
+        class A(Mixin, Base):
+            __tablename__ = 'a'
+            id = Column(Integer, primary_key=True)
+
+        class B(A):
+            pass
+
+        assert 'a' in A.__table__.c
+        assert 'b' in A.__table__.c
+
     def test_non_propagating_mixin(self):
 
         class NoJoinedTableNameMixin: