From: Mike Bayer Date: Sat, 28 Jul 2012 11:26:13 +0000 (-0400) Subject: - [bug] Fixes made to the constraints created/dropped X-Git-Tag: rel_0_3_6~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=50c7551d280fdaa099f15427b1627940181594f8;p=thirdparty%2Fsqlalchemy%2Falembic.git - [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 a66265d4..ef50e63d 100644 --- 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 2eef7cba..d044ccdc 100644 --- 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 9f785308..280003ca 100644 --- 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 58c560d6..e9799bc7 100644 --- 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) )