add checks for well-formed ColumnProperty. fixes #683

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index d3150c6cb0dbbc96e91442cdd8afa8e5eb13ab51..6ce9fd7069a5404b9732726b36565ed81343fcb3 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -17,6 +17,7 @@ from sqlalchemy.orm import session as sessionlib
 from sqlalchemy.orm import util as mapperutil
 import operator
 from sqlalchemy.orm.interfaces import StrategizedProperty, PropComparator
+from sqlalchemy.exceptions import ArgumentError
 
 __all__ = ['ColumnProperty', 'CompositeProperty', 'PropertyLoader', 'BackRef']
 
@@ -34,6 +35,12 @@ class ColumnProperty(StrategizedProperty):
         self.group = kwargs.pop('group', None)
         self.deferred = kwargs.pop('deferred', False)
         self.comparator = ColumnProperty.ColumnComparator(self)
+        # sanity check
+        for col in columns:
+            if not hasattr(col, 'name'):
+                if hasattr(col, 'label'):
+                    raise ArgumentError('ColumnProperties must be named for the mapper to work with them.  Try .label() to fix this')
+                raise ArgumentError('%r is not a valid candidate for ColumnProperty' % col)
         
     def create_strategy(self):
         if self.deferred:

diff --git a/test/orm/query.py b/test/orm/query.py
index ea5cc64def9bf3e52830807faf350b651ce20d7c..8943a6122fa2753712248f3a6c496008d489cf0e 100644 (file)
--- a/test/orm/query.py
+++ b/test/orm/query.py
@@ -742,10 +742,9 @@ class ExternalColumnsTest(QueryTest):
     def setup_mappers(self):
         pass
 
-    def test_external_columns(self):
-        """test querying mappings that reference external columns or selectables."""
-
-        f = (users.c.id *2).label('concat')
+    def test_external_columns_bad(self):
+        """test that SA catches some common mis-configurations of external columns."""
+        f = (users.c.id * 2)
         try:
             mapper(User, users, properties={
                 'concat': f,
@@ -753,10 +752,22 @@ class ExternalColumnsTest(QueryTest):
             class_mapper(User)
         except exceptions.ArgumentError, e:
             assert str(e) == "Column '%s' is not represented in mapper's table.  Use the `column_property()` function to force this column to be mapped as a read-only attribute." % str(f)
+        else:
+            raise 'expected ArgumentError'
         clear_mappers()
+        try:
+            mapper(User, users, properties={
+                'concat': column_property(users.c.id * 2),
+            })
+        except exceptions.ArgumentError, e:
+            assert str(e) == 'ColumnProperties must be named for the mapper to work with them.  Try .label() to fix this'
+        else:
+            raise 'expected ArgumentError'
 
+    def test_external_columns_good(self):
+        """test querying mappings that reference external columns or selectables."""
         mapper(User, users, properties={
-            'concat': column_property(f),
+            'concat': column_property((users.c.id * 2).label('concat')),
             'count': column_property(select([func.count(addresses.c.id)], users.c.id==addresses.c.user_id).correlate(users).label('count'))
         })