remove the alter_metadata feature

3 files changed, 40 insertions, 119 deletions

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 '<ColumnDelta altermetadata=%r, %s>' % (self.alter_metadata,
-            super(ColumnDelta, self).__repr__())
+        return '<ColumnDelta %s>' % 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
@@ -705,20 +705,6 @@ class TestColumnChange(fixture.DB):
             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'))