diff --git a/docs/changelog.rst b/docs/changelog.rst index f07cadb..732f703 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,7 +7,7 @@ Features - implemented columns adding with unique constraints for sqlite - implemented adding unique and foreign key constraints to columns for sqlite - +- remove experimental `alter_metadata` parameter Fixed bugs ****************** @@ -31,6 +31,7 @@ Fixed bugs differences in additon to missing columns and tables. - fixed bug when passing empty list in :func:`migrate.versioning.shell.main` failed +- #108: Fixed issues with firebird support. 0.6 (11.07.2010) --------------------------- diff --git a/migrate/changeset/databases/sqlite.py b/migrate/changeset/databases/sqlite.py index 01e48ef..447412d 100644 --- a/migrate/changeset/databases/sqlite.py +++ b/migrate/changeset/databases/sqlite.py @@ -86,10 +86,8 @@ class SQLiteColumnDropper(SQLiteHelper, ansisql.ANSIColumnDropper): ' from migration_tmp' def visit_column(self,column): - # For SQLite, we *have* to remove the column so the table + # For SQLite, we *have* to remove the column here so the table # is re-created properly. - # This violates the alter_metadata settting, but that - # is going away... column.remove_from_table(column.table,unset_table=False) super(SQLiteColumnDropper,self).visit_column(column) diff --git a/migrate/changeset/schema.py b/migrate/changeset/schema.py index 00e1a3e..8c4f458 100644 --- a/migrate/changeset/schema.py +++ b/migrate/changeset/schema.py @@ -29,9 +29,6 @@ __all__ = [ '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: @@ -188,11 +177,10 @@ class ColumnDelta(DictMixin, sqlalchemy.schema.SchemaItem): :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 +201,6 @@ class ColumnDelta(DictMixin, sqlalchemy.schema.SchemaItem): __visit_name__ = 'column' def __init__(self, *p, **kw): - self.alter_metadata = kw.pop("alter_metadata", False) self.meta = kw.pop("metadata", None) self.engine = kw.pop("engine", None) @@ -237,8 +224,7 @@ class ColumnDelta(DictMixin, sqlalchemy.schema.SchemaItem): self.apply_diffs(diffs) def __repr__(self): - return '' % (self.alter_metadata, - super(ColumnDelta, self).__repr__()) + return '' % super(ColumnDelta, self).__repr__() def __getitem__(self, key): if key not in self.keys(): @@ -314,7 +300,7 @@ class ColumnDelta(DictMixin, sqlalchemy.schema.SchemaItem): self.result_column.type = self.result_column.type() # add column to the table - if self.table is not None and self.alter_metadata: + if self.table is not None: self.result_column.add_to_table(self.table) def are_column_types_eq(self, old_type, new_type): @@ -376,38 +362,27 @@ class ColumnDelta(DictMixin, sqlalchemy.schema.SchemaItem): def _set_table(self, table): if isinstance(table, basestring): - if self.alter_metadata: - if not self.meta: - raise ValueError("metadata must be specified for table" - " reflection when using alter_metadata") - meta = self.meta - if self.engine: - meta.bind = self.engine - else: - if not self.engine and not self.meta: - raise ValueError("engine or metadata must be specified" - " to reflect tables") - if not self.engine: - self.engine = self.meta.bind - meta = sqlalchemy.MetaData(bind=self.engine) + if not self.engine and not self.meta: + raise ValueError("engine or metadata must be specified" + " to reflect tables") + if not self.meta: + self.meta = sqlalchemy.MetaData() + meta = self.meta + if self.engine: + meta.bind = self.engine self._table = sqlalchemy.Table(table, meta, autoload=True) elif isinstance(table, sqlalchemy.Table): 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) def _set_result_column(self, column): - """Set Column to Table based on alter_metadata evaluation.""" + """Set Column to Table.""" self.process_column(column) if not hasattr(self, 'current_name'): self.current_name = column.name - if self.alter_metadata: - self._result_column = column - else: - self._result_column = column.copy_fixed() + self._result_column = column table = property(_get_table, _set_table) result_column = property(_get_result_column, _set_result_column) @@ -456,22 +431,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 +481,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 +488,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,20 +517,16 @@ 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 visitorcallable = get_engine_visitor(engine, 'columndropper') engine._run_visitor(visitorcallable, self, connection, **kwargs) - if self.alter_metadata: - self.remove_from_table(self.table, unset_table=False) - self.table = None + self.remove_from_table(self.table, unset_table=False) + self.table = None return self def add_to_table(self, table): @@ -642,18 +605,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/migrate/tests/changeset/test_changeset.py b/migrate/tests/changeset/test_changeset.py index 1218b54..ce6fdcc 100644 --- a/migrate/tests/changeset/test_changeset.py +++ b/migrate/tests/changeset/test_changeset.py @@ -704,20 +704,6 @@ class TestColumnChange(fixture.DB): finally: cw.__exit__() - @fixture.usedb() - def test_alter_metadata(self): - """Test if alter_metadata is respected""" - - self.table.c.data.alter(type=String(100)) - - self.assert_(isinstance(self.table.c.data.type, String)) - self.assertEqual(self.table.c.data.type.length, 100) - - # nothing should change - self.table.c.data.alter(type=String(200),alter_metadata=False) - self.assert_(isinstance(self.table.c.data.type, String)) - self.assertEqual(self.table.c.data.type.length, 100) - @fixture.usedb() def test_alter_returns_delta(self): """Test if alter constructs return delta""" @@ -741,8 +727,7 @@ class TestColumnChange(fixture.DB): kw = dict(nullable=False, server_default='foobar', name='data_new', - type=String(50), - alter_metadata=True) + type=String(50)) if self.engine.name == 'firebird': del kw['nullable'] self.table.c.data.alter(**kw) @@ -805,13 +790,15 @@ class TestColumnDelta(fixture.DB): def test_deltas_two_columns(self): """Testing ColumnDelta with two columns""" - col_orig = self.mkcol(primary_key=True) - col_new = self.mkcol(name='ids', primary_key=True) - self.verify([], col_orig, col_orig) - self.verify(['name'], col_orig, col_orig, 'ids') - self.verify(['name'], col_orig, col_orig, name='ids') - self.verify(['name'], col_orig, col_new) - self.verify(['name', 'type'], col_orig, col_new, type=String) + col_orig = lambda :self.mkcol(primary_key=True) + col_new = lambda: self.mkcol(name='ids', primary_key=True) + # we have to create new columns, since comparing the ColumnDelta + # will apply differences + self.verify([], col_orig(), col_orig()) + self.verify(['name'], col_orig(), col_orig(), 'ids') + self.verify(['name'], col_orig(), col_orig(), name='ids') + self.verify(['name'], col_orig(), col_new()) + self.verify(['name', 'type'], col_orig(), col_new(), type=String) # Type comparisons self.verify([], self.mkcol(type=String), self.mkcol(type=String)) @@ -839,23 +826,16 @@ class TestColumnDelta(fixture.DB): self.verify([], self.mkcol(server_default='foobar'), self.mkcol('id', String, DefaultClause('foobar'))) self.verify(['type'], self.mkcol(server_default='foobar'), self.mkcol('id', Text, DefaultClause('foobar'))) - # test alter_metadata col = self.mkcol(server_default='foobar') - self.verify(['type'], col, self.mkcol('id', Text, DefaultClause('foobar')), alter_metadata=True) + self.verify(['type'], col, self.mkcol('id', Text, DefaultClause('foobar'))) self.assert_(isinstance(col.type, Text)) col = self.mkcol() - self.verify(['name', 'server_default', 'type'], col, self.mkcol('beep', Text, DefaultClause('foobar')), alter_metadata=True) + self.verify(['name', 'server_default', 'type'], col, self.mkcol('beep', Text, DefaultClause('foobar'))) self.assert_(isinstance(col.type, Text)) self.assertEqual(col.name, 'beep') self.assertEqual(col.server_default.arg, 'foobar') - col = self.mkcol() - self.verify(['name', 'server_default', 'type'], col, self.mkcol('beep', Text, DefaultClause('foobar')), alter_metadata=False) - self.assertFalse(isinstance(col.type, Text)) - self.assertNotEqual(col.name, 'beep') - self.assertFalse(col.server_default) - @fixture.usedb() def test_deltas_zero_columns(self): """Testing ColumnDelta with zero columns""" @@ -866,30 +846,20 @@ class TestColumnDelta(fixture.DB): self.verify(['type'], 'ids', table=self.table.name, type=String(80), engine=self.engine) self.verify(['type'], 'ids', table=self.table.name, type=String(80), metadata=self.meta) - # check if alter_metadata is respected self.meta.clear() - delta = self.verify(['type'], 'ids', table=self.table.name, type=String(80), alter_metadata=True, metadata=self.meta) + delta = self.verify(['type'], 'ids', table=self.table.name, type=String(80), metadata=self.meta) self.assert_(self.table.name in self.meta) self.assertEqual(delta.result_column.type.length, 80) self.assertEqual(self.meta.tables.get(self.table.name).c.ids.type.length, 80) - self.meta.clear() - self.verify(['type'], 'ids', table=self.table.name, type=String(80), alter_metadata=False, engine=self.engine) - self.assert_(self.table.name not in self.meta) - - self.meta.clear() - self.verify(['type'], 'ids', table=self.table.name, type=String(80), alter_metadata=False, metadata=self.meta) - self.assert_(self.table.name not in self.meta) - # test defaults self.meta.clear() - self.verify(['server_default'], 'ids', table=self.table.name, server_default='foobar', alter_metadata=True, metadata=self.meta) + self.verify(['server_default'], 'ids', table=self.table.name, server_default='foobar', metadata=self.meta) self.meta.tables.get(self.table.name).c.ids.server_default.arg == 'foobar' # test missing parameters self.assertRaises(ValueError, ColumnDelta, table=self.table.name) - self.assertRaises(ValueError, ColumnDelta, 'ids', table=self.table.name, alter_metadata=True) - self.assertRaises(ValueError, ColumnDelta, 'ids', table=self.table.name, alter_metadata=False) + self.assertRaises(ValueError, ColumnDelta, 'ids', table=self.table.name) def test_deltas_one_column(self): """Testing ColumnDelta with one column""" @@ -907,17 +877,11 @@ class TestColumnDelta(fixture.DB): self.assertEquals(delta.get('name'), 'blah') self.assertEquals(delta.current_name, 'id') - # check if alter_metadata is respected col_orig = self.mkcol(primary_key=True) - self.verify(['name', 'type'], col_orig, name='id12', type=Text, alter_metadata=True) + self.verify(['name', 'type'], col_orig, name='id12', type=Text) self.assert_(isinstance(col_orig.type, Text)) self.assertEqual(col_orig.name, 'id12') - col_orig = self.mkcol(primary_key=True) - self.verify(['name', 'type'], col_orig, name='id12', type=Text, alter_metadata=False) - self.assert_(isinstance(col_orig.type, String)) - self.assertEqual(col_orig.name, 'id') - # test server default col_orig = self.mkcol(primary_key=True) delta = self.verify(['server_default'], col_orig, DefaultClause('foobar'))