- [bug] Index will raise when arguments passed

cannot be interpreted as columns or expressions.
Will warn when Index is created
with no columns at all.  [ticket:2380]

diff --git a/CHANGES b/CHANGES
index be816002d68ba8d5db9ba88fb61d073d38a6fb63..90398f8f37a857660c05746bffe441213279a9fa 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -53,6 +53,11 @@ CHANGES
     key constraint of the reflected table.
     [ticket:2402]
 
+  - [bug] Index will raise when arguments passed
+    cannot be interpreted as columns or expressions.
+    Will warn when Index is created
+    with no columns at all.  [ticket:2380]
+
 - engine
   - [feature] Added pool_reset_on_return argument
     to create_engine, allows control over 

diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index 83a6e0f37d7051b84f1dd18235f30af6744db333..3ae0053c3344eee2509e800b455a9ac36d64e7e6 100644 (file)
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -2195,6 +2195,8 @@ class Index(ColumnCollectionMixin, SchemaItem):
         self.table = None
         # will call _set_parent() if table-bound column
         # objects are present
+        if not columns:
+            util.warn("No column names or expressions given for Index.")
         ColumnCollectionMixin.__init__(self, *columns)
         self.name = name
         self.unique = kw.pop('unique', False)
@@ -3007,9 +3009,11 @@ def _to_schema_column(element):
    return element
 
 def _to_schema_column_or_string(element):
-  if hasattr(element, '__clause_element__'):
-      element = element.__clause_element__()
-  return element
+    if hasattr(element, '__clause_element__'):
+        element = element.__clause_element__()
+    if not isinstance(element, (basestring, expression.ColumnElement)):
+        raise exc.ArgumentError("Element %r is not a string name or column element" % element)
+    return element
 
 class _CreateDropBase(DDLElement):
     """Base class for DDL constucts that represent CREATE and DROP or

diff --git a/test/sql/test_constraints.py b/test/sql/test_constraints.py
index 79a32d44ba627bbbc7e876bef9f3fa4302a14b11..246878624732793179f22827bc7e14b2f3276078 100644 (file)
--- a/test/sql/test_constraints.py
+++ b/test/sql/test_constraints.py
@@ -256,6 +256,39 @@ class ConstraintTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiled
                 Index('bar', t1.c.x)
         )
 
+    def test_raise_index_nonexistent_name(self):
+        m = MetaData()
+        # the KeyError isn't ideal here, a nicer message
+        # perhaps
+        assert_raises(
+            KeyError,
+            Table, 't', m, Column('x', Integer), Index("foo", "q")
+        )
+
+    def test_raise_not_a_column(self):
+        assert_raises(
+            exc.ArgumentError,
+            Index, "foo", 5
+        )
+
+    def test_warn_no_columns(self):
+        assert_raises_message(
+            exc.SAWarning,
+            "No column names or expressions given for Index.",
+            Index, "foo"
+        )
+
+    def test_raise_clauseelement_not_a_column(self):
+        m = MetaData()
+        t2 = Table('t2', m, Column('x', Integer))
+        class SomeClass(object):
+            def __clause_element__(self):
+                return t2
+        assert_raises(
+            exc.ArgumentError,
+            Index, "foo", SomeClass()
+        )
+
 class ConstraintCompilationTest(fixtures.TestBase, AssertsCompiledSQL):
     __dialect__ = 'default'
 

diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index 5f6c5065b322550edf3b4aa06260830bc9a256ab..ecbf8ad7545a07279af18ccda399519cb06f09ca 100644 (file)
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -655,7 +655,7 @@ class MetaDataTest(fixtures.TestBase, ComparesTables):
                 "Column('x', String(), table=<bar>), schema=None)"),
             (schema.DefaultGenerator(for_update=True), 
                 "DefaultGenerator(for_update=True)"),
-            (schema.Index("bar"), "Index('bar')"),
+            (schema.Index("bar", "c"), "Index('bar')"),
             (i1, "Index('bar', Column('x', Integer(), table=<foo>))"),
             (schema.FetchedValue(), "FetchedValue()"),
             (ck,