- Fixed bug in enhanced constraint-attachment logic introduced in

:ticket:`3341` where in the unusual case of a constraint that refers
to a mixture of :class:`.Column` objects and string column names
at the same time, the auto-attach-on-column-attach logic will be
skipped; for the constraint to be auto-attached in this case,
all columns must be assembled on the target table up front.
Added a new section to the migration document regarding the
original feature as well as this change.
fixes #3411

4 files changed, 186 insertions, 9 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 65d963029..a5703b2f6 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -19,6 +19,23 @@
     :version: 1.0.4
 
     .. change::
+        :tags: bug, schema
+        :tickets: 3411
+
+        Fixed bug in enhanced constraint-attachment logic introduced in
+        :ticket:`3341` where in the unusual case of a constraint that refers
+        to a mixture of :class:`.Column` objects and string column names
+        at the same time, the auto-attach-on-column-attach logic will be
+        skipped; for the constraint to be auto-attached in this case,
+        all columns must be assembled on the target table up front.
+        Added a new section to the migration document regarding the
+        original feature as well as this change.
+
+        .. seealso::
+
+            :ref:`change_3341`
+
+    .. change::
         :tags: bug, orm
         :tickets: 3409, 3320
 
@@ -551,6 +568,10 @@
         same time the columns are associated with the table.  This in particular
         helps in some edge cases in declarative but is also of general use.
 
+        .. seealso::
+
+            :ref:`change_3341`
+
     .. change::
         :tags: bug, sql
         :tickets: 3340
diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst
index e8b2161d0..4999e45de 100644
--- a/doc/build/changelog/migration_10.rst
+++ b/doc/build/changelog/migration_10.rst
@@ -8,7 +8,7 @@ What's New in SQLAlchemy 1.0?
     undergoing maintenance releases as of May, 2014,
     and SQLAlchemy version 1.0, released in April, 2015.
 
-    Document last updated: April 22, 2015
+    Document last updated: May 2, 2015
 
 Introduction
 ============
@@ -733,6 +733,95 @@ now make use of all CHECK constraint conventions.
 
 :ticket:`3299`
 
+.. _change_3341:
+
+Constraints referring to unattached Columns can auto-attach to the Table when their referred columns are attached
+-----------------------------------------------------------------------------------------------------------------
+
+Since at least version 0.8, a :class:`.Constraint` has had the ability to
+"auto-attach" itself to a :class:`.Table` based on being passed table-attached columns::
+
+    from sqlalchemy import Table, Column, MetaData, Integer, UniqueConstraint
+
+    m = MetaData()
+
+    t = Table('t', m,
+        Column('a', Integer),
+        Column('b', Integer)
+    )
+
+    uq = UniqueConstraint(t.c.a, t.c.b)  # will auto-attach to Table
+
+    assert uq in t.constraints
+
+In order to assist with some cases that tend to come up with declarative,
+this same auto-attachment logic can now function even if the :class:`.Column`
+objects are not yet associated with the :class:`.Table`; additional events
+are established such that when those :class:`.Column` objects are associated,
+the :class:`.Constraint` is also added::
+
+    from sqlalchemy import Table, Column, MetaData, Integer, UniqueConstraint
+
+    m = MetaData()
+
+    a = Column('a', Integer)
+    b = Column('b', Integer)
+
+    uq = UniqueConstraint(a, b)
+
+    t = Table('t', m, a, b)
+
+    assert uq in t.constraints  # constraint auto-attached
+
+The above feature was a late add as of version 1.0.0b3.  A fix as of
+version 1.0.4 for :ticket:`3411` ensures that this logic
+does not occur if the :class:`.Constraint` refers to a mixture of
+:class:`.Column` objects and string column names; as we do not yet have
+tracking for the addition of names to a :class:`.Table`::
+
+    from sqlalchemy import Table, Column, MetaData, Integer, UniqueConstraint
+
+    m = MetaData()
+
+    a = Column('a', Integer)
+    b = Column('b', Integer)
+
+    uq = UniqueConstraint(a, 'b')
+
+    t = Table('t', m, a, b)
+
+    # constraint *not* auto-attached, as we do not have tracking
+    # to locate when a name 'b' becomes available on the table
+    assert uq not in t.constraints
+
+Above, the attachment event for column "a" to table "t" will fire off before
+column "b" is attached (as "a" is stated in the :class:`.Table` constructor
+before "b"), and the constraint will fail to locate "b" if it were to attempt
+an attachment.  For consistency, if the constraint refers to any string names,
+the autoattach-on-column-attach logic is skipped.
+
+The original auto-attach logic of course remains in place, if the :class:`.Table`
+already contains all the target :class:`.Column` objects at the time
+the :class:`.Constraint` is constructed::
+
+    from sqlalchemy import Table, Column, MetaData, Integer, UniqueConstraint
+
+    m = MetaData()
+
+    a = Column('a', Integer)
+    b = Column('b', Integer)
+
+
+    t = Table('t', m, a, b)
+
+    uq = UniqueConstraint(a, 'b')
+
+    # constraint auto-attached normally as in older versions
+    assert uq in t.constraints
+
+
+:ticket:`3341`
+:ticket:`3411`
 
 .. _change_2051:
 
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index bbbd28b4d..e6d1d8858 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -2397,22 +2397,30 @@ class ColumnCollectionMixin(object):
             c for c in self._pending_colargs
             if isinstance(c, Column)
         ]
+
         cols_w_table = [
             c for c in col_objs if isinstance(c.table, Table)
         ]
+
         cols_wo_table = set(col_objs).difference(cols_w_table)
 
         if cols_wo_table:
+            # feature #3341 - place event listeners for Column objects
+            # such that when all those cols are attached, we autoattach.
             assert not evt, "Should not reach here on event call"
 
-            def _col_attached(column, table):
-                cols_wo_table.discard(column)
-                if not cols_wo_table:
-                    self._check_attach(evt=True)
-            self._cols_wo_table = cols_wo_table
-            for col in cols_wo_table:
-                col._on_table_attach(_col_attached)
-            return
+            # issue #3411 - don't do the per-column auto-attach if some of the
+            # columns are specified as strings.
+            has_string_cols = set(self._pending_colargs).difference(col_objs)
+            if not has_string_cols:
+                def _col_attached(column, table):
+                    cols_wo_table.discard(column)
+                    if not cols_wo_table:
+                        self._check_attach(evt=True)
+                self._cols_wo_table = cols_wo_table
+                for col in cols_wo_table:
+                    col._on_table_attach(_col_attached)
+                return
 
         columns = cols_w_table
 
diff --git a/test/sql/test_constraints.py b/test/sql/test_constraints.py
index 47f81a50c..3e8021ebe 100644
--- a/test/sql/test_constraints.py
+++ b/test/sql/test_constraints.py
@@ -1347,6 +1347,65 @@ class ConstraintAPITest(fixtures.TestBase):
             t2.append_column, c
         )
 
+    def test_auto_append_uq_on_col_attach_four(self):
+        """Test that a uniqueconstraint that names Column and string names
+        won't autoattach using deferred column attachment.
+
+        """
+        m = MetaData()
+
+        a = Column('a', Integer)
+        b = Column('b', Integer)
+        c = Column('c', Integer)
+        uq = UniqueConstraint(a, 'b', 'c')
+
+        t = Table('tbl', m, a)
+        assert uq not in t.constraints
+
+        t.append_column(b)
+        assert uq not in t.constraints
+
+        t.append_column(c)
+
+        # we don't track events for previously unknown columns
+        # named 'c' to be attached
+        assert uq not in t.constraints
+
+        t.append_constraint(uq)
+
+        assert uq in t.constraints
+
+        eq_(
+            [cn for cn in t.constraints if isinstance(cn, UniqueConstraint)],
+            [uq]
+        )
+
+    def test_auto_append_uq_on_col_attach_five(self):
+        """Test that a uniqueconstraint that names Column and string names
+        *will* autoattach if the table has all those names up front.
+
+        """
+        m = MetaData()
+
+        a = Column('a', Integer)
+        b = Column('b', Integer)
+        c = Column('c', Integer)
+
+        t = Table('tbl', m, a, c, b)
+
+        uq = UniqueConstraint(a, 'b', 'c')
+
+        assert uq in t.constraints
+
+        t.append_constraint(uq)
+
+        assert uq in t.constraints
+
+        eq_(
+            [cn for cn in t.constraints if isinstance(cn, UniqueConstraint)],
+            [uq]
+        )
+
     def test_index_asserts_cols_standalone(self):
         metadata = MetaData()