- [bug] Fixes made to the constraints created/dropped

  alongside so-called "schema" types such as
  Boolean and Enum.  The create/drop constraint logic
  does not kick in when using a dialect that doesn't
  use constraints for these types, such as postgresql,
  even when existing_type is specified to
  alter_column().  Additionally, the constraints
  are not affected if existing_type is passed but
  type_ is not, i.e. there's no net change
  in type.  #62

diff --git a/CHANGES b/CHANGES
index a66265d4b59786b7b5a1313384fe9a512cbec9fc..ef50e63d80bf5ecdf18b6057e8edc97f42e2635d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -15,6 +15,17 @@
   error message when the configuration is missing
   the 'script_directory' key.  #63
 
+- [bug] Fixes made to the constraints created/dropped
+  alongside so-called "schema" types such as
+  Boolean and Enum.  The create/drop constraint logic
+  does not kick in when using a dialect that doesn't
+  use constraints for these types, such as postgresql,
+  even when existing_type is specified to
+  alter_column().  Additionally, the constraints
+  are not affected if existing_type is passed but
+  type_ is not, i.e. there's no net change
+  in type.  #62
+
 0.3.5
 =====
 - [bug] Fixed issue whereby reflected server defaults

diff --git a/alembic/operations.py b/alembic/operations.py
index 2eef7cba1e020c101c5ca9d54adf64b663cb614f..d044ccdc7f2605f9fda73515463950a87c1f2941 100644 (file)
--- a/alembic/operations.py
+++ b/alembic/operations.py
@@ -220,12 +220,21 @@ class Operations(object):
          is not being changed; else MySQL sets this to NULL.
         """
 
-        if existing_type:
+        compiler = self.impl.dialect.statement_compiler(
+                            self.impl.dialect,
+                            None
+                        )
+        def _count_constraint(constraint):
+            return not isinstance(constraint, schema.PrimaryKeyConstraint) and \
+                (not constraint._create_rule or
+                    constraint._create_rule(compiler))
+
+        if existing_type and type_:
             t = self._table(table_name,
                         schema.Column(column_name, existing_type)
                     )
             for constraint in t.constraints:
-                if not isinstance(constraint, schema.PrimaryKeyConstraint):
+                if _count_constraint(constraint):
                     self.impl.drop_constraint(constraint)
 
         self.impl.alter_column(table_name, column_name,
@@ -241,7 +250,7 @@ class Operations(object):
         if type_:
             t = self._table(table_name, schema.Column(column_name, type_))
             for constraint in t.constraints:
-                if not isinstance(constraint, schema.PrimaryKeyConstraint):
+                if _count_constraint(constraint):
                     self.impl.add_constraint(constraint)
 
     def add_column(self, table_name, column):

diff --git a/tests/test_mssql.py b/tests/test_mssql.py
index 9f78530887e0de7da135b2c6429d27ed468467d8..280003cae639cf8c36260fe57d95995da44a5552 100644 (file)
--- a/tests/test_mssql.py
+++ b/tests/test_mssql.py
@@ -65,6 +65,14 @@ class OpTest(TestCase):
             'ALTER TABLE t ALTER COLUMN c INTEGER'
         )
 
+    def test_alter_column_dont_touch_constraints(self):
+        context = op_fixture('mssql')
+        from sqlalchemy import Boolean
+        op.alter_column('tests', 'col',
+            existing_type=Boolean(),
+            nullable=False)
+        context.assert_('ALTER TABLE tests ALTER COLUMN col BIT NOT NULL')
+
     def test_drop_index(self):
         context = op_fixture('mssql')
         op.drop_index('my_idx', 'my_table')
@@ -148,8 +156,8 @@ class OpTest(TestCase):
         context = op_fixture('mssql')
         op.alter_column("t", "c", name="c2", nullable=True, type_=Integer, server_default="5")
         context.assert_(
-            'ALTER TABLE t ALTER COLUMN c INTEGER NULL', 
-            "ALTER TABLE t ADD DEFAULT '5' FOR c", 
+            'ALTER TABLE t ALTER COLUMN c INTEGER NULL',
+            "ALTER TABLE t ADD DEFAULT '5' FOR c",
             "EXEC sp_rename 't.c', 'c2', 'COLUMN'"
         )
 

diff --git a/tests/test_op.py b/tests/test_op.py
index 58c560d6fbf38467665edf04f69dbfd11b3d6e07..e9799bc765be3c0f8bc4cd863aa63743c35d8000 100644 (file)
--- a/tests/test_op.py
+++ b/tests/test_op.py
@@ -41,17 +41,17 @@ def test_add_column_schema_type():
     context = op_fixture()
     op.add_column('t1', Column('c1', Boolean, nullable=False))
     context.assert_(
-        'ALTER TABLE t1 ADD COLUMN c1 BOOLEAN NOT NULL', 
+        'ALTER TABLE t1 ADD COLUMN c1 BOOLEAN NOT NULL',
         'ALTER TABLE t1 ADD CHECK (c1 IN (0, 1))'
     )
 
 def test_add_column_schema_type_checks_rule():
-    """Test that a schema type doesn't generate a 
+    """Test that a schema type doesn't generate a
     constraint based on check rule."""
     context = op_fixture('postgresql')
     op.add_column('t1', Column('c1', Boolean, nullable=False))
     context.assert_(
-        'ALTER TABLE t1 ADD COLUMN c1 BOOLEAN NOT NULL', 
+        'ALTER TABLE t1 ADD COLUMN c1 BOOLEAN NOT NULL',
     )
 
 def test_add_column_fk_self_referential():
@@ -66,7 +66,7 @@ def test_add_column_fk_schema():
     context = op_fixture()
     op.add_column('t1', Column('c1', Integer, ForeignKey('remote.t2.c2'), nullable=False))
     context.assert_(
-    'ALTER TABLE t1 ADD COLUMN c1 INTEGER NOT NULL', 
+    'ALTER TABLE t1 ADD COLUMN c1 INTEGER NOT NULL',
     'ALTER TABLE t1 ADD FOREIGN KEY(c1) REFERENCES remote.t2 (c2)'
     )
 
@@ -79,7 +79,7 @@ def test_alter_column_nullable():
     context = op_fixture()
     op.alter_column("t", "c", nullable=True)
     context.assert_(
-        # TODO: not sure if this is PG only or standard 
+        # TODO: not sure if this is PG only or standard
         # SQL
         "ALTER TABLE t ALTER COLUMN c DROP NOT NULL"
     )
@@ -88,7 +88,7 @@ def test_alter_column_not_nullable():
     context = op_fixture()
     op.alter_column("t", "c", nullable=False)
     context.assert_(
-        # TODO: not sure if this is PG only or standard 
+        # TODO: not sure if this is PG only or standard
         # SQL
         "ALTER TABLE t ALTER COLUMN c SET NOT NULL"
     )
@@ -153,9 +153,23 @@ def test_alter_column_schema_type_existing_type():
         'ALTER TABLE t ALTER COLUMN c VARCHAR(10)'
     )
 
+def test_alter_column_schema_type_existing_type_no_const():
+    context = op_fixture('postgresql')
+    op.alter_column("t", "c", type_=String(10), existing_type=Boolean(name="xyz"))
+    context.assert_(
+        'ALTER TABLE t ALTER COLUMN c TYPE VARCHAR(10)'
+    )
+
+def test_alter_column_schema_type_existing_type_no_new_type():
+    context = op_fixture('postgresql')
+    op.alter_column("t", "c", nullable=False, existing_type=Boolean(name="xyz"))
+    context.assert_(
+        'ALTER TABLE t ALTER COLUMN c SET NOT NULL'
+    )
+
 def test_add_foreign_key():
     context = op_fixture()
-    op.create_foreign_key('fk_test', 't1', 't2', 
+    op.create_foreign_key('fk_test', 't1', 't2',
                     ['foo', 'bar'], ['bat', 'hoho'])
     context.assert_(
         "ALTER TABLE t1 ADD CONSTRAINT fk_test FOREIGN KEY(foo, bar) "
@@ -249,7 +263,7 @@ def test_drop_table():
 def test_create_table_selfref():
     context = op_fixture()
     op.create_table(
-        "some_table", 
+        "some_table",
         Column('id', Integer, primary_key=True),
         Column('st_id', Integer, ForeignKey('some_table.id'))
     )
@@ -264,7 +278,7 @@ def test_create_table_selfref():
 def test_create_table_fk_and_schema():
     context = op_fixture()
     op.create_table(
-        "some_table", 
+        "some_table",
         Column('id', Integer, primary_key=True),
         Column('foo_id', Integer, ForeignKey('foo.id')),
         schema='schema'
@@ -280,7 +294,7 @@ def test_create_table_fk_and_schema():
 def test_create_table_two_fk():
     context = op_fixture()
     op.create_table(
-        "some_table", 
+        "some_table",
         Column('id', Integer, primary_key=True),
         Column('foo_id', Integer, ForeignKey('foo.id')),
         Column('foo_bar', Integer, ForeignKey('foo.bar')),
@@ -300,7 +314,7 @@ def test_inline_literal():
     from sqlalchemy.sql import table, column
     from sqlalchemy import String, Integer
 
-    account = table('account', 
+    account = table('account',
         column('name', String),
         column('id', Integer)
     )