Merge branch 'master' into rel_0_9

3 files changed, 57 insertions, 1 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 61a368dec..c18eedd73 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -15,6 +15,16 @@
     :version: 0.9.0b2
 
     .. change::
+        :tags: bug, orm, declarative
+        :tickets: 2828
+
+        Declarative does an extra check to detect if the same
+        :class:`.Column` is mapped multiple times under different properties
+        (which typically should be a :func:`.synonym` instead) or if two
+        or more :class:`.Column` objects are given the same name, raising
+        a warning if this condition is detected.
+
+    .. change::
         :tags: bug, firebird
         :tickets: 2898
 
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 820c0874b..f7668a540 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -14,7 +14,7 @@ from ... import util, exc
 from ...sql import expression
 from ... import event
 from . import clsregistry
-
+import collections
 
 def _declared_mapping_info(cls):
     # deferred mapping
@@ -173,15 +173,19 @@ def _as_declarative(cls, classname, dict_):
 
     # extract columns from the class dict
     declared_columns = set()
+    name_to_prop_key = collections.defaultdict(set)
     for key, c in list(our_stuff.items()):
         if isinstance(c, (ColumnProperty, CompositeProperty)):
             for col in c.columns:
                 if isinstance(col, Column) and \
                     col.table is None:
                     _undefer_column_name(key, col)
+                    if not isinstance(c, CompositeProperty):
+                        name_to_prop_key[col.name].add(key)
                     declared_columns.add(col)
         elif isinstance(c, Column):
             _undefer_column_name(key, c)
+            name_to_prop_key[c.name].add(key)
             declared_columns.add(c)
             # if the column is the same name as the key,
             # remove it from the explicit properties dict.
@@ -190,6 +194,15 @@ def _as_declarative(cls, classname, dict_):
             # in multi-column ColumnProperties.
             if key == c.key:
                 del our_stuff[key]
+
+    for name, keys in name_to_prop_key.items():
+        if len(keys) > 1:
+            util.warn(
+                "On class %r, Column object %r named directly multiple times, "
+                "only one will be used: %s" %
+                (classname, name, (", ".join(sorted(keys))))
+            )
+
     declared_columns = sorted(
         declared_columns, key=lambda c: c._creation_order)
     table = None
diff --git a/test/ext/declarative/test_basic.py b/test/ext/declarative/test_basic.py
index b119e356f..0d213fce3 100644
--- a/test/ext/declarative/test_basic.py
+++ b/test/ext/declarative/test_basic.py
@@ -143,6 +143,39 @@ class DeclarativeTest(DeclarativeTestBase):
         assert class_mapper(Bar).get_property('some_data').columns[0] \
             is t.c.data
 
+    def test_column_named_twice(self):
+        def go():
+            class Foo(Base):
+                __tablename__ = 'foo'
+
+                id = Column(Integer, primary_key=True)
+                x = Column('x', Integer)
+                y = Column('x', Integer)
+        assert_raises_message(
+            sa.exc.SAWarning,
+            "On class 'Foo', Column object 'x' named directly multiple times, "
+            "only one will be used: x, y",
+            go
+        )
+
+
+    def test_column_repeated_under_prop(self):
+        def go():
+            class Foo(Base):
+                __tablename__ = 'foo'
+
+                id = Column(Integer, primary_key=True)
+                x = Column('x', Integer)
+                y = column_property(x)
+                z = Column('x', Integer)
+
+        assert_raises_message(
+            sa.exc.SAWarning,
+            "On class 'Foo', Column object 'x' named directly multiple times, "
+            "only one will be used: x, y, z",
+            go
+        )
+
     def test_relationship_level_msg_for_invalid_callable(self):
         class A(Base):
             __tablename__ = 'a'