diff --git a/rhodecode/lib/dbmigrate/migrate/changeset/constraint.py b/rhodecode/lib/dbmigrate/migrate/changeset/constraint.py --- a/rhodecode/lib/dbmigrate/migrate/changeset/constraint.py +++ b/rhodecode/lib/dbmigrate/migrate/changeset/constraint.py @@ -28,7 +28,7 @@ class ConstraintChangeset(object): def __do_imports(self, visitor_name, *a, **kw): engine = kw.pop('engine', self.table.bind) from rhodecode.lib.dbmigrate.migrate.changeset.databases.visitor import (get_engine_visitor, - run_single_visitor) + run_single_visitor) visitorcallable = get_engine_visitor(engine, visitor_name) run_single_visitor(engine, visitorcallable, self, *a, **kw) diff --git a/rhodecode/lib/dbmigrate/migrate/changeset/databases/firebird.py b/rhodecode/lib/dbmigrate/migrate/changeset/databases/firebird.py --- a/rhodecode/lib/dbmigrate/migrate/changeset/databases/firebird.py +++ b/rhodecode/lib/dbmigrate/migrate/changeset/databases/firebird.py @@ -2,7 +2,7 @@ Firebird database specific implementations of changeset classes. """ from sqlalchemy.databases import firebird as sa_base - +from sqlalchemy.schema import PrimaryKeyConstraint from rhodecode.lib.dbmigrate.migrate import exceptions from rhodecode.lib.dbmigrate.migrate.changeset import ansisql, SQLA_06 @@ -27,13 +27,32 @@ class FBColumnDropper(ansisql.ANSIColumn if column.table.primary_key.columns.contains_column(column): column.table.primary_key.drop() # TODO: recreate primary key if it references more than this column - if column.unique or getattr(column, 'unique_name', None): - for cons in column.table.constraints: - if cons.contains_column(column): - cons.drop() - # TODO: recreate unique constraint if it refenrences more than this column - table = self.start_alter_table(column) + for index in column.table.indexes: + # "column in index.columns" causes problems as all + # column objects compare equal and return a SQL expression + if column.name in [col.name for col in index.columns]: + index.drop() + # TODO: recreate index if it references more than this column + + for cons in column.table.constraints: + if isinstance(cons,PrimaryKeyConstraint): + # will be deleted only when the column its on + # is deleted! + continue + + if SQLA_06: + should_drop = column.name in cons.columns + else: + should_drop = cons.contains_column(column) and cons.name + if should_drop: + self.start_alter_table(column) + self.append("DROP CONSTRAINT ") + self.append(self.preparer.format_constraint(cons)) + self.execute() + # TODO: recreate unique constraint if it refenrences more than this column + + self.start_alter_table(column) self.append('DROP %s' % self.preparer.format_column(column)) self.execute() diff --git a/rhodecode/lib/dbmigrate/migrate/changeset/databases/sqlite.py b/rhodecode/lib/dbmigrate/migrate/changeset/databases/sqlite.py --- a/rhodecode/lib/dbmigrate/migrate/changeset/databases/sqlite.py +++ b/rhodecode/lib/dbmigrate/migrate/changeset/databases/sqlite.py @@ -80,10 +80,17 @@ class SQLiteColumnDropper(SQLiteHelper, """SQLite ColumnDropper""" def _modify_table(self, table, column, delta): + columns = ' ,'.join(map(self.preparer.format_column, table.columns)) return 'INSERT INTO %(table_name)s SELECT ' + columns + \ ' from migration_tmp' + def visit_column(self,column): + # For SQLite, we *have* to remove the column here so the table + # is re-created properly. + column.remove_from_table(column.table,unset_table=False) + super(SQLiteColumnDropper,self).visit_column(column) + class SQLiteSchemaChanger(SQLiteHelper, ansisql.ANSISchemaChanger): """SQLite SchemaChanger""" diff --git a/rhodecode/lib/dbmigrate/migrate/changeset/schema.py b/rhodecode/lib/dbmigrate/migrate/changeset/schema.py --- a/rhodecode/lib/dbmigrate/migrate/changeset/schema.py +++ b/rhodecode/lib/dbmigrate/migrate/changeset/schema.py @@ -29,9 +29,6 @@ from rhodecode.lib.dbmigrate.migrate.cha 'ColumnDelta', ] -DEFAULT_ALTER_METADATA = True - - def create_column(column, table=None, *p, **kw): """Create a column, given the table. @@ -109,19 +106,11 @@ def alter_column(*p, **k): The :class:`~sqlalchemy.engine.base.Engine` to use for table reflection and schema alterations. - :param alter_metadata: - If `True`, which is the default, the - :class:`~sqlalchemy.schema.Column` will also modified. - If `False`, the :class:`~sqlalchemy.schema.Column` will be left - as it was. - :returns: A :class:`ColumnDelta` instance representing the change. """ - - k.setdefault('alter_metadata', DEFAULT_ALTER_METADATA) - + if 'table' not in k and isinstance(p[0], sqlalchemy.Column): k['table'] = p[0].table if 'engine' not in k: @@ -135,6 +124,12 @@ def alter_column(*p, **k): MigrateDeprecationWarning ) engine = k['engine'] + + # enough tests seem to break when metadata is always altered + # that this crutch has to be left in until they can be sorted + # out + k['alter_metadata']=True + delta = ColumnDelta(*p, **k) visitorcallable = get_engine_visitor(engine, 'schemachanger') @@ -188,11 +183,10 @@ class ColumnDelta(DictMixin, sqlalchemy. :param table: Table at which current Column should be bound to.\ If table name is given, reflection will be used. :type table: string or Table instance - :param alter_metadata: If True, it will apply changes to metadata. - :type alter_metadata: bool - :param metadata: If `alter_metadata` is true, \ - metadata is used to reflect table names into - :type metadata: :class:`MetaData` instance + + :param metadata: A :class:`MetaData` instance to store + reflected table names + :param engine: When reflecting tables, either engine or metadata must \ be specified to acquire engine object. :type engine: :class:`Engine` instance @@ -213,7 +207,11 @@ class ColumnDelta(DictMixin, sqlalchemy. __visit_name__ = 'column' def __init__(self, *p, **kw): + # 'alter_metadata' is not a public api. It exists purely + # as a crutch until the tests that fail when 'alter_metadata' + # behaviour always happens can be sorted out self.alter_metadata = kw.pop("alter_metadata", False) + self.meta = kw.pop("metadata", None) self.engine = kw.pop("engine", None) @@ -237,17 +235,19 @@ class ColumnDelta(DictMixin, sqlalchemy. self.apply_diffs(diffs) def __repr__(self): - return '' % (self.alter_metadata, - super(ColumnDelta, self).__repr__()) - + return '' % ( + self.alter_metadata, + super(ColumnDelta, self).__repr__() + ) + def __getitem__(self, key): if key not in self.keys(): - raise KeyError("No such diff key, available: %s" % self.diffs) + raise KeyError("No such diff key, available: %s" % self.diffs ) return getattr(self.result_column, key) def __setitem__(self, key, value): if key not in self.keys(): - raise KeyError("No such diff key, available: %s" % self.diffs) + raise KeyError("No such diff key, available: %s" % self.diffs ) setattr(self.result_column, key, value) def __delitem__(self, key): @@ -367,7 +367,7 @@ class ColumnDelta(DictMixin, sqlalchemy. for_update=True)) if toinit: column._init_items(*toinit) - + if not SQLA_06: column.args = [] @@ -395,7 +395,6 @@ class ColumnDelta(DictMixin, sqlalchemy. self._table = table if not self.alter_metadata: self._table.meta = sqlalchemy.MetaData(bind=self._table.bind) - def _get_result_column(self): return getattr(self, '_result_column', None) @@ -456,22 +455,18 @@ class ChangesetTable(object): :param name: New name of the table. :type name: string - :param alter_metadata: If True, table will be removed from metadata - :type alter_metadata: bool :param connection: reuse connection istead of creating new one. :type connection: :class:`sqlalchemy.engine.base.Connection` instance """ - self.alter_metadata = kwargs.pop('alter_metadata', DEFAULT_ALTER_METADATA) engine = self.bind self.new_name = name visitorcallable = get_engine_visitor(engine, 'schemachanger') run_single_visitor(engine, visitorcallable, self, connection, **kwargs) # Fix metadata registration - if self.alter_metadata: - self.name = name - self.deregister() - self._set_parent(self.metadata) + self.name = name + self.deregister() + self._set_parent(self.metadata) def _meta_key(self): return sqlalchemy.schema._get_table_key(self.name, self.schema) @@ -510,7 +505,6 @@ class ChangesetColumn(object): `~migrate.changeset.constraint.UniqueConstraint` on this column. :param primary_key_name: Creates :class:\ `~migrate.changeset.constraint.PrimaryKeyConstraint` on this column. - :param alter_metadata: If True, column will be added to table object. :param populate_default: If True, created column will be \ populated with defaults :param connection: reuse connection istead of creating new one. @@ -518,22 +512,19 @@ populated with defaults :type index_name: string :type unique_name: string :type primary_key_name: string - :type alter_metadata: bool :type populate_default: bool :type connection: :class:`sqlalchemy.engine.base.Connection` instance :returns: self """ self.populate_default = populate_default - self.alter_metadata = kwargs.pop('alter_metadata', DEFAULT_ALTER_METADATA) self.index_name = index_name self.unique_name = unique_name self.primary_key_name = primary_key_name for cons in ('index_name', 'unique_name', 'primary_key_name'): self._check_sanity_constraints(cons) - if self.alter_metadata: - self.add_to_table(table) + self.add_to_table(table) engine = self.table.bind visitorcallable = get_engine_visitor(engine, 'columngenerator') engine._run_visitor(visitorcallable, self, connection, **kwargs) @@ -550,59 +541,54 @@ populated with defaults ``ALTER TABLE DROP COLUMN``, for most databases. - :param alter_metadata: If True, column will be removed from table object. - :type alter_metadata: bool :param connection: reuse connection istead of creating new one. :type connection: :class:`sqlalchemy.engine.base.Connection` instance """ - self.alter_metadata = kwargs.pop('alter_metadata', DEFAULT_ALTER_METADATA) if table is not None: self.table = table engine = self.table.bind - if self.alter_metadata: - self.remove_from_table(self.table, unset_table=False) visitorcallable = get_engine_visitor(engine, 'columndropper') engine._run_visitor(visitorcallable, self, connection, **kwargs) - if self.alter_metadata: - self.table = None + self.remove_from_table(self.table, unset_table=False) + self.table = None return self def add_to_table(self, table): if table is not None and self.table is None: self._set_parent(table) - def _col_name_in_constraint(self, cons, name): + def _col_name_in_constraint(self,cons,name): return False - + def remove_from_table(self, table, unset_table=True): # TODO: remove primary keys, constraints, etc if unset_table: self.table = None - + to_drop = set() for index in table.indexes: columns = [] for col in index.columns: - if col.name != self.name: + if col.name!=self.name: columns.append(col) if columns: - index.columns = columns + index.columns=columns else: to_drop.add(index) table.indexes = table.indexes - to_drop - + to_drop = set() for cons in table.constraints: # TODO: deal with other types of constraint - if isinstance(cons, (ForeignKeyConstraint, + if isinstance(cons,(ForeignKeyConstraint, UniqueConstraint)): for col_name in cons.columns: - if not isinstance(col_name, basestring): + if not isinstance(col_name,basestring): col_name = col_name.name - if self.name == col_name: + if self.name==col_name: to_drop.add(cons) table.constraints = table.constraints - to_drop - + if table.c.contains_column(self): table.c.remove(self) @@ -643,18 +629,14 @@ class ChangesetIndex(object): :param name: New name of the Index. :type name: string - :param alter_metadata: If True, Index object will be altered. - :type alter_metadata: bool :param connection: reuse connection istead of creating new one. :type connection: :class:`sqlalchemy.engine.base.Connection` instance """ - self.alter_metadata = kwargs.pop('alter_metadata', DEFAULT_ALTER_METADATA) engine = self.table.bind self.new_name = name visitorcallable = get_engine_visitor(engine, 'schemachanger') engine._run_visitor(visitorcallable, self, connection, **kwargs) - if self.alter_metadata: - self.name = name + self.name = name class ChangesetDefaultClause(object): diff --git a/rhodecode/lib/dbmigrate/migrate/versioning/genmodel.py b/rhodecode/lib/dbmigrate/migrate/versioning/genmodel.py --- a/rhodecode/lib/dbmigrate/migrate/versioning/genmodel.py +++ b/rhodecode/lib/dbmigrate/migrate/versioning/genmodel.py @@ -111,12 +111,12 @@ class ModelGenerator(object): out.append(")") return out - def _get_tables(self, missingA=False, missingB=False, modified=False): + def _get_tables(self,missingA=False,missingB=False,modified=False): to_process = [] - for bool_, names, metadata in ( - (missingA, self.diff.tables_missing_from_A, self.diff.metadataB), - (missingB, self.diff.tables_missing_from_B, self.diff.metadataA), - (modified, self.diff.tables_different, self.diff.metadataA), + for bool_,names,metadata in ( + (missingA,self.diff.tables_missing_from_A,self.diff.metadataB), + (missingB,self.diff.tables_missing_from_B,self.diff.metadataA), + (modified,self.diff.tables_different,self.diff.metadataA), ): if bool_: for name in names: @@ -140,7 +140,7 @@ class ModelGenerator(object): decls = ['from rhodecode.lib.dbmigrate.migrate.changeset import schema', 'meta = MetaData()'] for table in self._get_tables( - missingA=True, missingB=True, modified=True + missingA=True,missingB=True,modified=True ): decls.extend(self.getTableDefn(table)) @@ -169,11 +169,11 @@ class ModelGenerator(object): modelTable, col.name)) for modelCol, databaseCol, modelDecl, databaseDecl in diffDecl: upgradeCommands.append( - 'assert False, "Can\'t alter columns: %s:%s=>%s"', - modelTable, modelCol.name, databaseCol.name) + 'assert False, "Can\'t alter columns: %s:%s=>%s"' % ( + modelTable, modelCol.name, databaseCol.name)) downgradeCommands.append( - 'assert False, "Can\'t alter columns: %s:%s=>%s"', - modelTable, modelCol.name, databaseCol.name) + 'assert False, "Can\'t alter columns: %s:%s=>%s"' % ( + modelTable, modelCol.name, databaseCol.name)) pre_command = ' meta.bind = migrate_engine' return ( @@ -181,7 +181,7 @@ class ModelGenerator(object): '\n'.join([pre_command] + ['%s%s' % (indent, line) for line in upgradeCommands]), '\n'.join([pre_command] + ['%s%s' % (indent, line) for line in downgradeCommands])) - def _db_can_handle_this_change(self, td): + def _db_can_handle_this_change(self,td): if (td.columns_missing_from_B and not td.columns_missing_from_A and not td.columns_different): @@ -207,9 +207,9 @@ class ModelGenerator(object): dbTable = self.diff.metadataB.tables[tableName] td = self.diff.tables_different[tableName] - + if self._db_can_handle_this_change(td): - + for col in td.columns_missing_from_B: modelTable.columns[col].create() for col in td.columns_missing_from_A: diff --git a/rhodecode/lib/dbmigrate/migrate/versioning/script/py.py b/rhodecode/lib/dbmigrate/migrate/versioning/script/py.py --- a/rhodecode/lib/dbmigrate/migrate/versioning/script/py.py +++ b/rhodecode/lib/dbmigrate/migrate/versioning/script/py.py @@ -4,6 +4,7 @@ import shutil import warnings import logging +import inspect from StringIO import StringIO from rhodecode.lib.dbmigrate import migrate @@ -49,7 +50,7 @@ class PythonScript(base.BaseScript): :returns: Upgrade / Downgrade script :rtype: string """ - + if isinstance(repository, basestring): # oh dear, an import cycle! from rhodecode.lib.dbmigrate.migrate.versioning.repository import Repository @@ -65,7 +66,7 @@ class PythonScript(base.BaseScript): excludeTables=[repository.version_table]) # TODO: diff can be False (there is no difference?) decls, upgradeCommands, downgradeCommands = \ - genmodel.ModelGenerator(diff, engine).toUpgradeDowngradePython() + genmodel.ModelGenerator(diff,engine).toUpgradeDowngradePython() # Store differences into file. src = Template(opts.pop('templates_path', None)).get_script(opts.pop('templates_theme', None)) @@ -136,12 +137,12 @@ class PythonScript(base.BaseScript): funcname = base.operations[op] script_func = self._func(funcname) - try: - script_func(engine) - except TypeError: - warnings.warn("upgrade/downgrade functions must accept engine" - " parameter (since version > 0.5.4)", MigrateDeprecationWarning) - raise + # check for old way of using engine + if not inspect.getargspec(script_func)[0]: + raise TypeError("upgrade/downgrade functions must accept engine" + " parameter (since version 0.5.4)") + + script_func(engine) @property def module(self): diff --git a/rhodecode/lib/dbmigrate/migrate/versioning/script/sql.py b/rhodecode/lib/dbmigrate/migrate/versioning/script/sql.py --- a/rhodecode/lib/dbmigrate/migrate/versioning/script/sql.py +++ b/rhodecode/lib/dbmigrate/migrate/versioning/script/sql.py @@ -18,6 +18,7 @@ class SqlScript(base.BaseScript): :returns: :class:`SqlScript instance `""" cls.require_notfound(path) + src = Template(opts.pop('templates_path', None)).get_sql_script(theme=opts.pop('templates_theme', None)) shutil.copy(src, path) return cls(path) diff --git a/rhodecode/lib/dbmigrate/migrate/versioning/shell.py b/rhodecode/lib/dbmigrate/migrate/versioning/shell.py --- a/rhodecode/lib/dbmigrate/migrate/versioning/shell.py +++ b/rhodecode/lib/dbmigrate/migrate/versioning/shell.py @@ -77,8 +77,7 @@ def main(argv=None, **kwargs): %s Enter "%%prog help COMMAND" for information on a particular command. - """ % '\n\t'.join(["%s - %s" % (command.ljust(28), - api.command_desc.get(command)) for command in commands]) + """ % '\n\t'.join(["%s - %s" % (command.ljust(28), api.command_desc.get(command)) for command in commands]) parser = PassiveOptionParser(usage=usage) parser.add_option("-d", "--debug", diff --git a/rhodecode/lib/dbmigrate/migrate/versioning/template.py b/rhodecode/lib/dbmigrate/migrate/versioning/template.py --- a/rhodecode/lib/dbmigrate/migrate/versioning/template.py +++ b/rhodecode/lib/dbmigrate/migrate/versioning/template.py @@ -80,7 +80,7 @@ class Template(pathed.Pathed): def get_repository(self, *a, **kw): """Calls self._get_item('repository', *a, **kw)""" return self._get_item('repository', *a, **kw) - + def get_script(self, *a, **kw): """Calls self._get_item('script', *a, **kw)""" return self._get_item('script', *a, **kw)