From: Jonathan Ellis <jbellis@gmail.com>
Date: Wed, 25 Jul 2007 21:17:48 +0000 (+0000)
Subject: add checks for well-formed ColumnProperty.  fixes #683
X-Git-Tag: rel_0_4_6~24
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7b64c4c259a279973043bd85e05ada0a2db241ab;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

add checks for well-formed ColumnProperty.  fixes #683
---

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index d3150c6cb0..6ce9fd7069 100644
--- 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 ea5cc64def..8943a6122f 100644
--- 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'))
         })