From 2485118c24b2293747dfafb3be58a6bdc65f7d66 Mon Sep 17 00:00:00 2001 From: Roman Podolyaka Date: Fri, 18 Oct 2013 14:23:11 +0300 Subject: [PATCH] Fix dropping of indexed columns in sqlite/sa08 Version 0.8 of SQLAlchemy added support of indexes on expressions in addition to plain table columns, which changed the way indexes are created. This broke support of dropping columns of composite indexes for SQLite: due to limitations of ALTER in SQLite every time a column is dropped, we recreate the whole table without the given column; if a column is a part of a composite index, we change the index definition to omit that column and then indexes are recreated too. SQLAlchemy versions starting from 0.8 no more pay attention to 'columns' attribute of Index instances when generating DDL for indexes, so when one of columns of a composite index is dropped, we try to create a new index on the column that doesn't exist anymore, which of course fails. Closes-Bug: #1241038 Change-Id: I777b8ce36e36f49bfb0889908811a063cf1a527b --- migrate/changeset/__init__.py | 1 + migrate/changeset/schema.py | 6 ++-- migrate/tests/changeset/test_changeset.py | 35 ++++++++++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/migrate/changeset/__init__.py b/migrate/changeset/__init__.py index 80ea152..256d704 100644 --- a/migrate/changeset/__init__.py +++ b/migrate/changeset/__init__.py @@ -14,6 +14,7 @@ warnings.simplefilter('always', DeprecationWarning) _sa_version = tuple(int(re.match("\d+", x).group(0)) for x in _sa_version.split(".")) SQLA_07 = _sa_version >= (0, 7) +SQLA_08 = _sa_version >= (0, 8) del re del _sa_version diff --git a/migrate/changeset/schema.py b/migrate/changeset/schema.py index c467cc5..5a77208 100644 --- a/migrate/changeset/schema.py +++ b/migrate/changeset/schema.py @@ -11,7 +11,7 @@ from sqlalchemy.schema import ForeignKeyConstraint from sqlalchemy.schema import UniqueConstraint from migrate.exceptions import * -from migrate.changeset import SQLA_07 +from migrate.changeset import SQLA_07, SQLA_08 from migrate.changeset.databases.visitor import (get_engine_visitor, run_single_visitor) @@ -573,7 +573,9 @@ populated with defaults if col.name!=self.name: columns.append(col) if columns: - index.columns=columns + index.columns = columns + if SQLA_08: + index.expressions = columns else: to_drop.add(index) table.indexes = table.indexes - to_drop diff --git a/migrate/tests/changeset/test_changeset.py b/migrate/tests/changeset/test_changeset.py index 3989ea3..bd7f034 100644 --- a/migrate/tests/changeset/test_changeset.py +++ b/migrate/tests/changeset/test_changeset.py @@ -18,6 +18,7 @@ class TestAddDropColumn(fixture.DB): """ level = fixture.DB.CONNECT table_name = 'tmp_adddropcol' + table_name_idx = 'tmp_adddropcol_idx' table_int = 0 def _setup(self, url): @@ -26,14 +27,27 @@ class TestAddDropColumn(fixture.DB): self.table = Table(self.table_name, self.meta, Column('id', Integer, unique=True), ) + self.table_idx = Table( + self.table_name_idx, + self.meta, + Column('id', Integer, primary_key=True), + Column('a', Integer), + Column('b', Integer), + Index('test_idx', 'a', 'b') + ) self.meta.bind = self.engine if self.engine.has_table(self.table.name): self.table.drop() + if self.engine.has_table(self.table_idx.name): + self.table_idx.drop() self.table.create() + self.table_idx.create() def _teardown(self): if self.engine.has_table(self.table.name): self.table.drop() + if self.engine.has_table(self.table_idx.name): + self.table_idx.drop() self.meta.clear() super(TestAddDropColumn,self)._teardown() @@ -257,7 +271,26 @@ class TestAddDropColumn(fixture.DB): col.drop(self.table) -# TODO: remove already attached columns with indexes, uniques, pks, fks .. +# TODO: remove already attached columns with uniques, pks, fks .. + @fixture.usedb(not_supported='postgresql') + def test_drop_column_of_composite_index(self): + # NOTE(rpodolyaka): postgresql automatically drops a composite index + # if one of its columns is dropped + self.table_idx.c.b.drop() + + reflected = Table(self.table_idx.name, MetaData(), autoload=True, + autoload_with=self.engine) + index = next(iter(reflected.indexes)) + self.assertEquals(['a'], [c.name for c in index.columns]) + + @fixture.usedb() + def test_drop_all_columns_of_composite_index(self): + self.table_idx.c.a.drop() + self.table_idx.c.b.drop() + + reflected = Table(self.table_idx.name, MetaData(), autoload=True, + autoload_with=self.engine) + self.assertEquals(0, len(reflected.indexes)) def _check_index(self,expected): if 'mysql' in self.engine.name or 'postgres' in self.engine.name: