From: Mike Bayer Date: Tue, 23 Jan 2018 17:38:28 +0000 (-0500) Subject: Detect indexes for table that's dropped X-Git-Tag: rel_0_9_8~6^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=058b2eb4873e3331700080ebd141d9980ffa3e3d;p=thirdparty%2Fsqlalchemy%2Falembic.git Detect indexes for table that's dropped Fixed bug where the indexes would not be included in a migration that was dropping the owning table. The fix now will also emit DROP INDEX for the indexes ahead of time, but more importantly will include CREATE INDEX in the downgrade migration. Change-Id: I15852bf1cca6de26dd6e7e5ede69bc9e2145c5c0 Fixes: #468 --- diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index 3abb32df..1b660613 100644 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@ -136,6 +136,16 @@ def _compare_tables(conn_table_names, metadata_table_names, _compat_autogen_column_reflect(inspector)) inspector.reflecttable(t, None) if autogen_context.run_filters(t, tname, "table", True, None): + + modify_table_ops = ops.ModifyTableOps(tname, [], schema=s) + + comparators.dispatch("table")( + autogen_context, modify_table_ops, + s, tname, t, None + ) + if not modify_table_ops.is_empty(): + upgrade_ops.ops.append(modify_table_ops) + upgrade_ops.ops.append( ops.DropTableOp.from_table(t) ) @@ -362,13 +372,18 @@ def _compare_indexes_and_uniques( inspector = autogen_context.inspector is_create_table = conn_table is None + is_drop_table = metadata_table is None # 1a. get raw indexes and unique constraints from metadata ... - metadata_unique_constraints = set( - uq for uq in metadata_table.constraints - if isinstance(uq, sa_schema.UniqueConstraint) - ) - metadata_indexes = set(metadata_table.indexes) + if metadata_table is not None: + metadata_unique_constraints = set( + uq for uq in metadata_table.constraints + if isinstance(uq, sa_schema.UniqueConstraint) + ) + metadata_indexes = set(metadata_table.indexes) + else: + metadata_unique_constraints = set() + metadata_indexes = set() conn_uniques = conn_indexes = frozenset() @@ -401,8 +416,13 @@ def _compare_indexes_and_uniques( # 2. convert conn-level objects from raw inspector records # into schema objects - conn_uniques = set(_make_unique_constraint(uq_def, conn_table) - for uq_def in conn_uniques) + if is_drop_table: + # for DROP TABLE uniques are inline, don't need them + conn_uniques = set() + else: + conn_uniques = set(_make_unique_constraint(uq_def, conn_table) + for uq_def in conn_uniques) + conn_indexes = set(_make_index(ix, conn_table) for ix in conn_indexes) # 2a. if the dialect dupes unique indexes as unique constraints @@ -493,7 +513,7 @@ def _compare_indexes_and_uniques( # can't report unique indexes as added if we don't # detect them return - if is_create_table: + if is_create_table or is_drop_table: # unique constraints are created inline with table defs return if autogen_context.run_filters( @@ -523,6 +543,10 @@ def _compare_indexes_and_uniques( log.info( "Detected removed index '%s' on '%s'", obj.name, tname) else: + if is_create_table or is_drop_table: + # if the whole table is being dropped, we don't need to + # consider unique constraint separately + return if autogen_context.run_filters( obj.const, obj.name, "unique_constraint", True, None): @@ -617,7 +641,6 @@ def _correct_for_uq_duplicates_uix( conn_indexes, metadata_unique_constraints, metadata_indexes): - # dedupe unique indexes vs. constraints, since MySQL / Oracle # doesn't really have unique constraints as a separate construct. # but look in the metadata and try to maintain constructs @@ -644,6 +667,7 @@ def _correct_for_uq_duplicates_uix( (cons.name, cons) for cons in conn_unique_constraints if cons.info['duplicates_index'] ) + for overlap in uqs_dupe_indexes: if overlap not in metadata_uq_names: if _uq_constraint_sig(uqs_dupe_indexes[overlap]).sig \ @@ -775,7 +799,7 @@ def _compare_foreign_keys( # if we're doing CREATE TABLE, all FKs are created # inline within the table def - if conn_table is None: + if conn_table is None or metadata_table is None: return inspector = autogen_context.inspector diff --git a/docs/build/unreleased/468.rst b/docs/build/unreleased/468.rst new file mode 100644 index 00000000..3abc974e --- /dev/null +++ b/docs/build/unreleased/468.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, autogenerate + :tickets: 468 + + Fixed bug where the indexes would not be included in a + migration that was dropping the owning table. The fix + now will also emit DROP INDEX for the indexes ahead of time, + but more importantly will include CREATE INDEX in the + downgrade migration. diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index db77d745..85f87a31 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -514,7 +514,10 @@ class AutogenerateDiffTest(ModelOne, AutogenTest, TestBase): ) eq_( [(rec[0], rec[1].name) for rec in uo.as_diffs()], - [('remove_table', 'extra'), ('remove_table', 'user')] + [ + ('remove_table', 'extra'), + ('remove_index', 'pw_idx'), + ('remove_table', 'user'), ] ) diff --git a/tests/test_autogen_indexes.py b/tests/test_autogen_indexes.py index 2eaf7d68..00793007 100644 --- a/tests/test_autogen_indexes.py +++ b/tests/test_autogen_indexes.py @@ -424,6 +424,65 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase): diffs = self._fixture(m1, m2) eq_(diffs[0][0], 'remove_index') + def test_drop_table_w_indexes(self): + m1 = MetaData() + m2 = MetaData() + + t = Table( + 'some_table', m1, + Column('id', Integer, primary_key=True), + Column('x', String(20)), + Column('y', String(20)), + ) + Index('xy_idx', t.c.x, t.c.y) + Index('y_idx', t.c.y) + + diffs = self._fixture(m1, m2) + eq_(diffs[0][0], 'remove_index') + eq_(diffs[1][0], 'remove_index') + eq_(diffs[2][0], 'remove_table') + + eq_( + set([diffs[0][1].name, diffs[1][1].name]), + set(['xy_idx', 'y_idx']) + ) + + # this simply doesn't fully work before we had + # effective deduping of indexes/uniques. + @config.requirements.sqlalchemy_100 + def test_drop_table_w_uq_constraint(self): + m1 = MetaData() + m2 = MetaData() + + Table( + 'some_table', m1, + Column('id', Integer, primary_key=True), + Column('x', String(20)), + Column('y', String(20)), + UniqueConstraint('y', name='uq_y') + ) + + diffs = self._fixture(m1, m2) + + if self.reports_unique_constraints_as_indexes: + # for MySQL this UQ will look like an index, so + # make sure it at least sets it up correctly + eq_(diffs[0][0], 'remove_index') + eq_(diffs[1][0], 'remove_table') + eq_(len(diffs), 2) + + constraints = [c for c in diffs[1][1].constraints + if isinstance(c, UniqueConstraint)] + eq_(len(constraints), 0) + else: + eq_(diffs[0][0], 'remove_table') + eq_(len(diffs), 1) + + constraints = [c for c in diffs[0][1].constraints + if isinstance(c, UniqueConstraint)] + if self.reports_unique_constraints: + eq_(len(constraints), 1) + def test_unnamed_cols_changed(self): m1 = MetaData() m2 = MetaData()