use sqlalchemy preparer to do SQL quote formatting. this is a raw change, tests are yet to be written

migrate/changeset/ansisql.py

@@ -31,10 +31,6 @@ class RawAlterTableVisitor(object):
             ret = ret.fullname
         return ret
 
-    def _do_quote_table_identifier(self, identifier):
-        """Returns a quoted version of the given table identifier."""
-        return '"%s"' % identifier
-
     def start_alter_table(self, param):
         """Returns the start of an ``ALTER TABLE`` SQL-Statement.
 
@@ -47,9 +43,7 @@ class RawAlterTableVisitor(object):
           or string (table name)
         """
         table = self._to_table(param)
-        table_name = self._to_table_name(table)
+        self.append('\nALTER TABLE %s ' % self.preparer.format_table(table))
-        self.append('\nALTER TABLE %s ' % \
-                        self._do_quote_table_identifier(table_name))
         return table
 
     def _pk_constraint(self, table, column, status):
@@ -107,7 +101,8 @@ class ANSIColumnGenerator(AlterTableVisitor, SchemaGenerator):
 
 class ANSIColumnDropper(AlterTableVisitor):
     """Extends ANSI SQL dropper for column dropping (``ALTER TABLE
-    DROP COLUMN``)."""
+    DROP COLUMN``).
+    """
 
     def visit_column(self, column):
         """Drop a column from its table.
@@ -116,8 +111,7 @@ class ANSIColumnDropper(AlterTableVisitor):
         :type column: :class:`sqlalchemy.Column`
         """
         table = self.start_alter_table(column)
-        self.append(' DROP COLUMN %s' % \
+        self.append(' DROP COLUMN %s' % self.preparer.format_column(column))
-                        self._do_quote_column_identifier(column.name))
         self.execute()
 
 
@@ -136,18 +130,11 @@ class ANSISchemaChanger(AlterTableVisitor, SchemaGenerator):
     name. NONE means the name is unchanged.
     """
 
-    def _do_quote_column_identifier(self, identifier):
-        """override this function to define how identifiers (table and
-        column names) should be written in the SQL.  For instance, in
-        PostgreSQL, double quotes should surround the identifier
-        """
-        return identifier
-
     def visit_table(self, param):
         """Rename a table. Other ops aren't supported."""
         table, newname = param
         self.start_alter_table(table)
-        self.append("RENAME TO %s"%newname)
+        self.append("RENAME TO %s" % self.preparer.quote(newname, table.quote))
         self.execute()
 
     def visit_column(self, delta):
@@ -200,8 +187,8 @@ class ANSISchemaChanger(AlterTableVisitor, SchemaGenerator):
         nullable = delta['nullable']
         table = self._to_table(delta)
         self.start_alter_table(table_name)
-        self.append("ALTER COLUMN %s " % \
+        # TODO: use preparer.format_column
-                        self._do_quote_column_identifier(col_name))
+        self.append("ALTER COLUMN %s " % self.preparer.quote_identifier(col_name))
         if nullable:
             self.append("DROP NOT NULL")
         else:
@@ -214,9 +201,10 @@ class ANSISchemaChanger(AlterTableVisitor, SchemaGenerator):
         dummy = sa.Column(None, None, server_default=server_default)
         default_text = self.get_column_default_string(dummy)
         self.start_alter_table(table_name)
-        self.append("ALTER COLUMN %s " % \
+        # TODO: use preparer.format_column
-                        self._do_quote_column_identifier(col_name))
+        self.append("ALTER COLUMN %s " % self.preparer.quote_identifier(col_name))
         if default_text is not None:
+            # TODO: format needed?
             self.append("SET DEFAULT %s" % default_text)
         else:
             self.append("DROP DEFAULT")
@@ -229,21 +217,25 @@ class ANSISchemaChanger(AlterTableVisitor, SchemaGenerator):
             type = type()
         type_text = type.dialect_impl(self.dialect).get_col_spec()
         self.start_alter_table(table_name)
-        self.append("ALTER COLUMN %s TYPE %s" % \
+        # TODO: does type need formating?
-                        (self._do_quote_column_identifier(col_name),
+        # TODO: use preparer.format_column
-                         type_text))
+        self.append("ALTER COLUMN %s TYPE %s" %
+            (self.preparer.quote_identifier(col_name), type_text))
 
     def _visit_column_name(self, table_name, col_name, delta):
         new_name = delta['name']
         self.start_alter_table(table_name)
+        # TODO: use preparer.format_column
         self.append('RENAME COLUMN %s TO %s' % \
-                        (self._do_quote_column_identifier(col_name),
+                        (self.preparer.quote_identifier(col_name),
-                         self._do_quote_column_identifier(new_name)))
+                         self.preparer.quote_identifier(new_name)))
 
     def visit_index(self, param):
         """Rename an index; #36"""
         index, newname = param
-        self.append("ALTER INDEX %s RENAME TO %s" % (index.name, newname))
+        self.append("ALTER INDEX %s RENAME TO %s" %
+            (self.preparer.quote(self._validate_identifier(index.name, True), index.quote),
+             self.preparer.quote(self._validate_identifier(newname, True) , index.quote)))
         self.execute()
 
 
@@ -269,23 +261,23 @@ class ANSIConstraintCommon(AlterTableVisitor):
             ret = cons.name
         else:
             ret = cons.name = cons.autoname()
-        return ret
+        return self.preparer.quote(ret, cons.quote)
 
 
 class ANSIConstraintGenerator(ANSIConstraintCommon):
 
     def get_constraint_specification(self, cons, **kwargs):
         if isinstance(cons, constraint.PrimaryKeyConstraint):
-            col_names = ','.join([i.name for i in cons.columns])
+            col_names = ', '.join([self.preparer.format_column(col) for col in cons.columns])
             ret = "PRIMARY KEY (%s)" % col_names
             if cons.name:
                 # Named constraint
-                ret = ("CONSTRAINT %s " % cons.name)+ret
+                ret = ("CONSTRAINT %s " % self.preparer.format_constraint(cons)) + ret
         elif isinstance(cons, constraint.ForeignKeyConstraint):
             params = dict(
-                columns=','.join([c.name for c in cons.columns]),
+                columns = ', '.join(map(self.preparer.format_column, cons.columns)),
-                reftable=cons.reftable,
+                reftable = self.preparer.format_table(cons.reftable),
-                referenced=','.join([c.name for c in cons.referenced]),
+                referenced = ', '.join(map(self.preparer.format_column, cons.referenced)),
                 name = self.get_constraint_name(cons),
             )
             ret = "CONSTRAINT %(name)s FOREIGN KEY (%(columns)s) "\

migrate/changeset/databases/mysql.py

@@ -10,19 +10,11 @@ MySQLSchemaGenerator = sa_base.MySQLSchemaGenerator
 
 
 class MySQLColumnGenerator(MySQLSchemaGenerator, ansisql.ANSIColumnGenerator):
-
-    def _do_quote_table_identifier(self, identifier):
-        return '%s'%identifier
     pass
 
 
 class MySQLColumnDropper(ansisql.ANSIColumnDropper):
-
+    pass
-    def _do_quote_table_identifier(self, identifier):
-        return '%s'%identifier
-
-    def _do_quote_column_identifier(self, identifier):
-        return '%s'%identifier
 
 
 class MySQLSchemaChanger(MySQLSchemaGenerator, ansisql.ANSISchemaChanger):
@@ -49,9 +41,10 @@ class MySQLSchemaChanger(MySQLSchemaGenerator, ansisql.ANSISchemaChanger):
         if not column.table:
             column.table = delta.table
         colspec = self.get_column_specification(column)
-        self.start_alter_table(table_name)
+        # TODO: we need table formating here
+        self.start_alter_table(self.preparer.quote(table_name, True))
         self.append("CHANGE COLUMN ")
-        self.append(col_name)
+        self.append(self.preparer.quote(col_name, True))
         self.append(' ')
         self.append(colspec)
 
@@ -59,14 +52,9 @@ class MySQLSchemaChanger(MySQLSchemaGenerator, ansisql.ANSISchemaChanger):
         # If MySQL can do this, I can't find how
         raise exceptions.NotSupportedError("MySQL cannot rename indexes")
 
-    def _do_quote_table_identifier(self, identifier):
-        return '%s'%identifier
-
 
 class MySQLConstraintGenerator(ansisql.ANSIConstraintGenerator):
-
+    pass
-    def _do_quote_table_identifier(self, identifier):
-        return '%s'%identifier
 
 
 class MySQLConstraintDropper(ansisql.ANSIConstraintDropper):
@@ -85,12 +73,9 @@ class MySQLConstraintDropper(ansisql.ANSIConstraintDropper):
     def visit_migrate_foreign_key_constraint(self, constraint):
         self.start_alter_table(constraint)
         self.append("DROP FOREIGN KEY ")
-        self.append(constraint.name)
+        self.append(self.preparer.format_constraint(constraint))
         self.execute()
 
-    def _do_quote_table_identifier(self, identifier):
-        return '%s'%identifier
-
 
 class MySQLDialect(ansisql.ANSIDialect):
     columngenerator = MySQLColumnGenerator

migrate/changeset/databases/oracle.py

@@ -76,7 +76,8 @@ class OracleSchemaChanger(OracleSchemaGenerator, ansisql.ANSISchemaChanger):
         if dropdefault_hack:
             column.server_default = None
 
-        self.start_alter_table(table_name)
+        # TODO: format from table
+        self.start_alter_table(self.preparer.quote(table_name, True))
         self.append("MODIFY ")
         self.append(colspec)
 

migrate/changeset/databases/postgres.py

@@ -11,40 +11,27 @@ from sqlalchemy.databases import postgres as sa_base
 PGSchemaGenerator = sa_base.PGSchemaGenerator
 
 
-class PGSchemaGeneratorMixin(object):
+class PGColumnGenerator(PGSchemaGenerator, ansisql.ANSIColumnGenerator):
-    """Common code used by the PostgreSQL specific classes."""
-
-    def _do_quote_table_identifier(self, identifier):
-        return identifier
-
-    def _do_quote_column_identifier(self, identifier):
-        return '"%s"'%identifier
-
-
-class PGColumnGenerator(PGSchemaGenerator, ansisql.ANSIColumnGenerator,
-                        PGSchemaGeneratorMixin):
     """PostgreSQL column generator implementation."""
     pass
 
 
-class PGColumnDropper(ansisql.ANSIColumnDropper, PGSchemaGeneratorMixin):
+class PGColumnDropper(ansisql.ANSIColumnDropper):
     """PostgreSQL column dropper implementation."""
     pass
 
 
-class PGSchemaChanger(ansisql.ANSISchemaChanger, PGSchemaGeneratorMixin):
+class PGSchemaChanger(ansisql.ANSISchemaChanger):
     """PostgreSQL schema changer implementation."""
     pass
 
 
-class PGConstraintGenerator(ansisql.ANSIConstraintGenerator,
+class PGConstraintGenerator(ansisql.ANSIConstraintGenerator):
-                            PGSchemaGeneratorMixin):
     """PostgreSQL constraint generator implementation."""
     pass
 
 
-class PGConstraintDropper(ansisql.ANSIConstraintDropper,
+class PGConstraintDropper(ansisql.ANSIConstraintDropper):
-                          PGSchemaGeneratorMixin):
     """PostgreSQL constaint dropper implementation."""
     pass
 

migrate/changeset/databases/sqlite.py

@@ -19,7 +19,7 @@ class SQLiteHelper(object):
         except:
             table = self._to_table(param)
             raise
-        table_name = self._to_table_name(table)
+        table_name = self.preparer.format_table(table)
         self.append('ALTER TABLE %s RENAME TO migration_tmp' % table_name)
         self.execute()
 
@@ -41,7 +41,7 @@ class SQLiteColumnDropper(SQLiteHelper, ansisql.ANSIColumnDropper):
 
     def _modify_table(self, table, column):
         del table.columns[column.name]
-        columns = ','.join([c.name for c in table.columns])
+        columns = ' ,'.join(map(self.preparer.format_column, table.columns))
         return 'INSERT INTO %(table_name)s SELECT ' + columns + \
             ' from migration_tmp'
 
@@ -61,17 +61,14 @@ class SQLiteSchemaChanger(SQLiteHelper, ansisql.ANSISchemaChanger):
     def visit_index(self, param):
         self._not_supported('ALTER INDEX')
 
-    def _do_quote_column_identifier(self, identifier):
-        return '"%s"'%identifier
-
 
 class SQLiteConstraintGenerator(ansisql.ANSIConstraintGenerator):
 
     def visit_migrate_primary_key_constraint(self, constraint):
         tmpl = "CREATE UNIQUE INDEX %s ON %s ( %s )"
-        cols = ','.join([c.name for c in constraint.columns])
+        cols = ', '.join(map(self.preparer.format_column, constraint.columns))
-        tname = constraint.table.name
+        tname = self.preparer.format_table(constraint.table)
-        name = constraint.name
+        name = self.get_constraint_name(constraint)
         msg = tmpl % (name, tname, cols)
         self.append(msg)
         self.execute()
@@ -88,11 +85,11 @@ class SQLiteFKGenerator(SQLiteSchemaChanger, ansisql.ANSIFKGenerator):
             self.execute()
 
 
-class SQLiteConstraintDropper(ansisql.ANSIColumnDropper):
+class SQLiteConstraintDropper(ansisql.ANSIColumnDropper, ansisql.ANSIConstraintCommon):
 
     def visit_migrate_primary_key_constraint(self, constraint):
         tmpl = "DROP INDEX %s "
-        name = constraint.name
+        name = self.get_constraint_name(constraint)
         msg = tmpl % (name)
         self.append(msg)
         self.execute()

migrate/changeset/databases/visitor.py

@@ -18,7 +18,14 @@ dialects = {
 def get_engine_visitor(engine, name):
     """
     Get the visitor implementation for the given database engine.
+
+    :param engine: SQLAlchemy Engine
+    :param name: Name of the visitor
+    :type name: string
+    :type engine: Engine
+    :returns: visitor
     """
+    # TODO: link to supported visitors
     return get_dialect_visitor(engine.dialect, name)
 
 
@@ -28,7 +35,16 @@ def get_dialect_visitor(sa_dialect, name):
 
     Finds the visitor implementation based on the dialect class and
     returns and instance initialized with the given name.
+
+    Binds dialect specific preparer to visitor.
     """
+
+    # map sa dialect to migrate dialect and return visitor
     sa_dialect_cls = sa_dialect.__class__
     migrate_dialect_cls = dialects[sa_dialect_cls]
-    return migrate_dialect_cls.visitor(name)
+    visitor = migrate_dialect_cls.visitor(name)
+
+    # bind preparer
+    visitor.preparer = sa_dialect.preparer(sa_dialect)
+
+    return visitor

migrate/changeset/exceptions.py

@@ -7,18 +7,15 @@ class Error(Exception):
     """
     Changeset error.
     """
-    pass
 
 
 class NotSupportedError(Error):
     """
     Not supported error.
     """
-    pass
 
 
 class InvalidConstraintError(Error):
     """
     Invalid constraint error.
     """
-    pass

migrate/changeset/schema.py

@@ -17,14 +17,20 @@ __all__ = [
 
 
 def create_column(column, table=None, *p, **k):
-    """Create a column, given the table"""
+    """Create a column, given the table
+    
+    API to :meth:`column.create`
+    """
     if table is not None:
         return table.create_column(column, *p, **k)
     return column.create(*p, **k)
 
 
 def drop_column(column, table=None, *p, **k):
-    """Drop a column, given the table"""
+    """Drop a column, given the table
+    
+    API to :meth:`column.drop`
+    """
     if table is not None:
         return table.drop_column(column, *p, **k)
     return column.drop(*p, **k)
@@ -32,7 +38,10 @@ def drop_column(column, table=None, *p, **k):
 
 def rename_table(table, name, engine=None):
     """Rename a table, given the table's current name and the new
-    name."""
+    name.
+    
+    API to :meth:`table.rename`
+    """
     table = _to_table(table, engine)
     table.rename(name)
 
@@ -43,6 +52,8 @@ def rename_index(index, name, table=None, engine=None):
     Takes an index name/object, a table name/object, and an
     engine. Engine and table aren't required if an index object is
     given.
+
+    API to :meth:`index.rename`
     """
     index = _to_index(index, table, engine)
     index.rename(name)
@@ -52,6 +63,8 @@ def alter_column(*p, **k):
 
     Parameters: column name, table name, an engine, and the properties
     of that column to change
+
+    API to :meth:`column.alter`
     """
     if len(p) and isinstance(p[0], sqlalchemy.Column):
         col = p[0]
@@ -170,6 +183,7 @@ class _ColumnDelta(dict):
         # Things are initialized differently depending on how many column
         # parameters are given. Figure out how many and call the appropriate
         # method.
+
         if len(p) >= 1 and isinstance(p[0], sqlalchemy.Column):
             # At least one column specified
             if len(p) >= 2 and isinstance(p[1], sqlalchemy.Column):
@@ -183,25 +197,28 @@ class _ColumnDelta(dict):
             func = self._init_0col
         diffs = func(*p, **k)
         self._set_diffs(diffs)
-    # Column attributes that can be altered
-    diff_keys = ('name', 'type', 'nullable', 'default', 'server_default',
-                 'primary_key', 'foreign_key')
 
-    def _get_table_name(self):
+    # Column attributes that can be altered
+    diff_keys = ('name',
+                 'type',
+                 'nullable',
+                 'default',
+                 'server_default',
+                 'primary_key',
+                 'foreign_key')
+
+    @property
+    def table_name(self):
         if isinstance(self._table, basestring):
             ret = self._table
         else:
             ret = self._table.name
         return ret
-    table_name = property(_get_table_name)
 
-    def _get_table(self):
+    @property
-        if isinstance(self._table, basestring):
+    def table(self):
-            ret = None
+        if isinstance(self._table, sqlalchemy.Table):
-        else:
+            return self._table
-            ret = self._table
-        return ret
-    table = property(_get_table)
 
     def _init_0col(self, current_name, *p, **k):
         p, k = self._init_normalize_params(p, k)

test/changeset/test_changeset.py

@@ -1,12 +1,19 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
 import sqlalchemy
 from sqlalchemy import *
-from test import fixture
-from migrate import changeset
-from migrate.changeset import *
-from migrate.changeset.schema import _ColumnDelta
 from sqlalchemy.databases import information_schema
 
 import migrate
+from migrate import changeset
+from migrate.changeset import *
+from migrate.changeset.schema import _ColumnDelta
+
+from test import fixture
+
+
+# TODO: add sqlite unique constraints (indexes), test quoting
 
 class TestAddDropColumn(fixture.DB):
     level = fixture.DB.CONNECT

test/changeset/test_constraint.py

@@ -1,13 +1,20 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
 from sqlalchemy import *
 from sqlalchemy.util import *
-from test import fixture
+
 from migrate.changeset import *
 
+from test import fixture
+
 class TestConstraint(fixture.DB):
     level = fixture.DB.CONNECT
+
     def _setup(self, url):
         super(TestConstraint, self)._setup(url)
         self._create_table()
+
     def _teardown(self):
         if hasattr(self, 'table') and self.engine.has_table(self.table.name):
             self.table.drop()
@@ -19,15 +26,14 @@ class TestConstraint(fixture.DB):
         self.table = Table('mytable', self.meta,
             Column('id', Integer),
             Column('fkey', Integer),
-            mysql_engine='InnoDB'
+            mysql_engine='InnoDB')
-        )
         if self.engine.has_table(self.table.name):
             self.table.drop()
         self.table.create()
-        #self.assertEquals(self.table.primary_key,[])
         self.assertEquals(len(self.table.primary_key), 0)
         self.assert_(isinstance(self.table.primary_key,
             schema.PrimaryKeyConstraint), self.table.primary_key.__class__)
+
     def _define_pk(self, *cols):
         # Add a pk by creating a PK constraint
         pk = PrimaryKeyConstraint(table=self.table, *cols)
@@ -129,6 +135,7 @@ class TestAutoname(fixture.DB):
         cons = PrimaryKeyConstraint(self.table.c.id)
         cons.create()
         self.refresh_table()
+        # TODO: test for index for sqlite
         if not self.url.startswith('sqlite'):
             self.assertEquals(list(cons.columns),list(self.table.primary_key))