- [feature] Conflicts between columns on

    single-inheritance declarative subclasses,
    with or without using a mixin, can be resolved
    using a new @declared_attr usage described
    in the documentation.  [ticket:2472]

3 files changed, 133 insertions, 20 deletions

diff --git a/test/ext/declarative/test_basic.py b/test/ext/declarative/test_basic.py
index 2d6534942..5af2b88dc 100644
--- a/test/ext/declarative/test_basic.py
+++ b/test/ext/declarative/test_basic.py
@@ -897,26 +897,24 @@ class DeclarativeTest(DeclarativeTestBase):
         eq_(sess.query(User).all(), [User(name='u1', address_count=2,
             addresses=[Address(email='one'), Address(email='two')])])
 
-    def test_useless_declared_attr_warns_on_subclass(self):
-        def go():
-            class MyBase(Base):
-                __tablename__ = 'foo'
-                id = Column(Integer, primary_key=True)
-                @declared_attr
-                def somecol(cls):
-                    return Column(Integer)
+    def test_declared_on_base_class(self):
+        class MyBase(Base):
+            __tablename__ = 'foo'
+            id = Column(Integer, primary_key=True)
+            @declared_attr
+            def somecol(cls):
+                return Column(Integer)
 
-            class MyClass(MyBase):
-                __tablename__ = 'bar'
-        assert_raises_message(
-            sa.exc.SAWarning,
-            r"Regular \(i.e. not __special__\) attribute 'MyBase.somecol' "
-            "uses @declared_attr, but owning class "
-            "<class 'test.ext.declarative..*test_basic..*MyBase'> is "
-            "mapped - not applying to subclass <class "
-            "'test.ext.declarative..*test_basic..*MyClass'>.",
-            go
-        )
+        class MyClass(MyBase):
+            __tablename__ = 'bar'
+            id = Column(Integer, ForeignKey('foo.id'), primary_key=True)
+
+        # previously, the 'somecol' declared_attr would be ignored
+        # by the mapping and would remain unused.  now we take
+        # it as part of MyBase.
+
+        assert 'somecol' in MyBase.__table__.c
+        assert 'somecol' not in MyClass.__table__.c
 
     def test_column(self):
 
diff --git a/test/ext/declarative/test_inheritance.py b/test/ext/declarative/test_inheritance.py
index 3bcd17bb7..7a2cb2def 100644
--- a/test/ext/declarative/test_inheritance.py
+++ b/test/ext/declarative/test_inheritance.py
@@ -478,6 +478,66 @@ class DeclarativeInheritanceTest(DeclarativeTestBase):
         eq_(sess.query(Engineer).filter_by(primary_language='cobol'
             ).one(), Engineer(name='vlad', primary_language='cobol'))
 
+    def test_columns_single_inheritance_conflict_resolution(self):
+        """Test that a declared_attr can return the existing column and it will
+        be ignored.  this allows conditional columns to be added.
+
+        See [ticket:2472].
+
+        """
+        class Person(Base):
+            __tablename__ = 'person'
+            id = Column(Integer, primary_key=True)
+
+        class Engineer(Person):
+            """single table inheritance"""
+
+            @declared_attr
+            def target_id(cls):
+                return cls.__table__.c.get('target_id',
+                       Column(Integer, ForeignKey('other.id'))
+                    )
+            @declared_attr
+            def target(cls):
+                return relationship("Other")
+
+        class Manager(Person):
+            """single table inheritance"""
+
+            @declared_attr
+            def target_id(cls):
+                return cls.__table__.c.get('target_id',
+                        Column(Integer, ForeignKey('other.id'))
+                    )
+            @declared_attr
+            def target(cls):
+                return relationship("Other")
+
+        class Other(Base):
+            __tablename__ = 'other'
+            id = Column(Integer, primary_key=True)
+
+        is_(
+            Engineer.target_id.property.columns[0],
+            Person.__table__.c.target_id
+        )
+        is_(
+            Manager.target_id.property.columns[0],
+            Person.__table__.c.target_id
+        )
+        # do a brief round trip on this
+        Base.metadata.create_all()
+        session = Session()
+        o1, o2 = Other(), Other()
+        session.add_all([
+            Engineer(target=o1),
+            Manager(target=o2),
+            Manager(target=o1)
+            ])
+        session.commit()
+        eq_(session.query(Engineer).first().target, o1)
+
+
     def test_joined_from_single(self):
 
         class Company(Base, fixtures.ComparableEntity):
diff --git a/test/ext/declarative/test_mixin.py b/test/ext/declarative/test_mixin.py
index 0876ebe63..a77d6be81 100644
--- a/test/ext/declarative/test_mixin.py
+++ b/test/ext/declarative/test_mixin.py
@@ -1,5 +1,5 @@
 from test.lib.testing import eq_, assert_raises, \
-    assert_raises_message
+    assert_raises_message, is_
 from sqlalchemy.ext import declarative as decl
 import sqlalchemy as sa
 from test.lib import testing
@@ -13,6 +13,8 @@ from sqlalchemy.util import classproperty
 from sqlalchemy.ext.declarative import declared_attr
 from test.lib import fixtures
 
+Base = None
+
 class DeclarativeTestBase(fixtures.TestBase, testing.AssertsExecutionResults):
     def setup(self):
         global Base
@@ -287,6 +289,59 @@ class DeclarativeMixinTest(DeclarativeTestBase):
         assert len(General.bar.prop.columns) == 1
         assert Specific.bar.prop is General.bar.prop
 
+    def test_columns_single_inheritance_conflict_resolution(self):
+        """Test that a declared_attr can return the existing column and it will
+        be ignored.  this allows conditional columns to be added.
+
+        See [ticket:2472].
+
+        """
+        class Person(Base):
+            __tablename__ = 'person'
+            id = Column(Integer, primary_key=True)
+
+        class Mixin(object):
+            @declared_attr
+            def target_id(cls):
+                return cls.__table__.c.get('target_id',
+                        Column(Integer, ForeignKey('other.id'))
+                    )
+
+            @declared_attr
+            def target(cls):
+                return relationship("Other")
+
+        class Engineer(Mixin, Person):
+            """single table inheritance"""
+
+        class Manager(Mixin, Person):
+            """single table inheritance"""
+
+        class Other(Base):
+            __tablename__ = 'other'
+            id = Column(Integer, primary_key=True)
+
+        is_(
+            Engineer.target_id.property.columns[0],
+            Person.__table__.c.target_id
+        )
+        is_(
+            Manager.target_id.property.columns[0],
+            Person.__table__.c.target_id
+        )
+        # do a brief round trip on this
+        Base.metadata.create_all()
+        session = Session()
+        o1, o2 = Other(), Other()
+        session.add_all([
+            Engineer(target=o1),
+            Manager(target=o2),
+            Manager(target=o1)
+            ])
+        session.commit()
+        eq_(session.query(Engineer).first().target, o1)
+
+
     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.