From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 27 Dec 2013 22:10:55 +0000 (-0500)
Subject: - Declarative does an extra check to detect if the same
X-Git-Tag: rel_0_9_0~8^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=de786a4208e621229769a8fb1f876f358dc4e70e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- 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. [ticket:2828]
---

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 61a368decf..c18eedd734 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -14,6 +14,16 @@
 .. changelog::
     :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 820c0874be..f7668a5401 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 b119e356f7..0d213fce3d 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'