206_add_instance_cleaned.py fails to downgrade with SQLite 3

[Thomas Goirand]

When running:

./run_tests.sh -N -P nova.tests.db.test_migrations.TestNovaMigrations.test_walk_versions

there's a bad failure when running the test against SQLite 3. It seems to be located 206_add_instance_cleaned.py, line 47, in downgrade.

Traceback (most recent call last):
  File "/home/zigo/sources/openstack/havana/nova/build-area/nova-2013.2~rc2/nova/tests/db/test_migrations.py", line 159, in test_walk_versions
    self._walk_versions(engine, self.snake_walk)
  File "/home/zigo/sources/openstack/havana/nova/build-area/nova-2013.2~rc2/nova/tests/db/test_migrations.py", line 381, in _walk_versions
    engine, version - 1, with_data=True)
  File "/home/zigo/sources/openstack/havana/nova/build-area/nova-2013.2~rc2/nova/tests/db/test_migrations.py", line 398, in _migrate_down
    self.migration_api.downgrade(engine, self.REPOSITORY, version)
  File "/usr/lib/python2.7/dist-packages/migrate/versioning/api.py", line 202, in downgrade
    return _migrate(url, repository, version, upgrade=False, err=err, **opts)
  File "<string>", line 2, in _migrate
  File "/home/zigo/sources/openstack/havana/nova/build-area/nova-2013.2~rc2/nova/db/sqlalchemy/migration.py", line 40, in patched_with_engine
    return f(*a, **kw)
  File "/usr/lib/python2.7/dist-packages/migrate/versioning/api.py", line 366, in _migrate
    schema.runchange(ver, change, changeset.step)
  File "/usr/lib/python2.7/dist-packages/migrate/versioning/schema.py", line 91, in runchange
    change.run(self.engine, step)
  File "/usr/lib/python2.7/dist-packages/migrate/versioning/script/py.py", line 145, in run
    script_func(engine)
  File "/home/zigo/sources/openstack/havana/nova/build-area/nova-2013.2~rc2/nova/db/sqlalchemy/migrate_repo/versions/206_add_instance_cleaned.py", line 47, in downgrade
    instances.columns.cleaned.drop()
  File "/usr/lib/python2.7/dist-packages/migrate/changeset/schema.py", line 549, in drop
    engine._run_visitor(visitorcallable, self, connection, **kwargs)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 1479, in _run_visitor
    conn._run_visitor(visitorcallable, element, **kwargs)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 1122, in _run_visitor
    **kwargs).traverse_single(element)
  File "/usr/lib/python2.7/dist-packages/migrate/changeset/ansisql.py", line 53, in traverse_single
    ret = super(AlterTableVisitor, self).traverse_single(elem)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/sql/visitors.py", line 111, in traverse_single
    return meth(obj, **kw)
  File "/usr/lib/python2.7/dist-packages/migrate/changeset/databases/sqlite.py", line 90, in visit_column
    super(SQLiteColumnDropper,self).visit_column(column)
  File "/usr/lib/python2.7/dist-packages/migrate/changeset/databases/sqlite.py", line 53, in visit_column
    self.recreate_table(table,column,delta)
  File "/usr/lib/python2.7/dist-packages/migrate/changeset/databases/sqlite.py", line 40, in recreate_table
    table.create(bind=self.connection)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/schema.py", line 614, in create
    checkfirst=checkfirst)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 1122, in _run_visitor
    **kwargs).traverse_single(element)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/sql/visitors.py", line 111, in traverse_single
    return meth(obj, **kw)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/ddl.py", line 93, in visit_table
    self.traverse_single(index)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/sql/visitors.py", line 111, in traverse_single
    return meth(obj, **kw)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/ddl.py", line 105, in visit_index
    self.connection.execute(schema.CreateIndex(index))
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 662, in execute
    params)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 720, in _execute_ddl
    compiled
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 874, in _execute_context
    context)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 1024, in _handle_dbapi_exception
    exc_info
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/util/compat.py", line 195, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 867, in _execute_context
    context)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/default.py", line 324, in do_execute
    cursor.execute(statement, parameters)
OperationalError: (OperationalError) table instances has no column named cleaned u'CREATE INDEX instances_host_deleted_cleaned_idx ON instances (host, deleted, cleaned)' ()

[Roman Podoliaka (rpodolyaka)]

This is has nothing to do with Nova, but rather with sqlalchemy-migrate not supporting SQLAlchemy >= 0.8 versions properly.

Due to limitation of ALTER statement in SQLite sqlachemy-migrate recreates a table when its column is dropped. If there is an index on this column - it's not recreated for a new table. For composite indexes it's a bit interesting: sqlalchemy-migrate authors tried to recreate composite indexes not including the column that was dropped. This worked fine till SQLAlchemy 0.8 was released, which brought support of expression indexes. sqlalchemy-migrate doesn't know nothing about expression columns, so it just ignores them. At the same time, SQLAlchemy versions >= 0.8 use expressions attribute of Index instance to emit DDL. This means, that sqlalchemy-migrate forces SQLAlchemy to create an index on column, that doesn't exist.

I'll check, if I can fix this easily in sqlalchemy-migrate, so we could release later. Anyway, I wouldn't recommend anyone to use migrations with SQLite.

[Thomas Goirand (thomas-goirand)]

We've discussed this already, and the way to fix it was to re-create a temporary table as a copy of the old table, then drop the old table, and rename the temporary table. Probably this should be done directly in sqlalchemy-migrate so we don't have to think about it.

[Thomas Goirand (thomas-goirand)]

Oh, when I think about it, one solution for this one could be to simply not run the downgrade unit test when running with SQLite. We shouldn't care too much about that one, IMO.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/52634

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/52634
Committed: http://github.com/stackforge/sqlalchemy-migrate/commit/2485118c24b2293747dfafb3be58a6bdc65f7d66
Submitter: Jenkins
Branch: master

commit 2485118c24b2293747dfafb3be58a6bdc65f7d66
Author: Roman Podolyaka <email address hidden>
Date: Fri Oct 18 14:23:11 2013 +0300

    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