F. Gabriel Gosselin avatar F. Gabriel Gosselin committed 7bfc69a

Refactored the delete_column for MySQL to use a decorator (and rely on parent delete)
Added corresponding unit test to verify functionality (fails when decorator removed)

Comments (0)

Files changed (3)

south/db/generic.py

         return 'INVALID'
 
 class DatabaseOperations(object):
-
     """
     Generic SQL implementation of the DatabaseOperations.
     Some of this code comes from Django Evolution.

south/db/mysql.py

+# MySQL-specific implementations for south
+# Original author: Andrew Godwin
+# Patches by: F. Gabriel Gosselin <gabrielNOSPAM@evidens.ca>
+#             aarranz
 
 from django.db import connection
 from django.conf import settings
 from south.db import generic
 
+from south.logger import get_logger
+
+def delete_column_constraints(func):
+    """
+    Decorates column operation functions for MySQL.
+    Deletes the constraints from the database and clears local cache.
+    """
+    def _column_rm(self, table_name, column_name, *args, **opts):
+        try:
+            self.delete_foreign_key(table_name, column_name)
+        except ValueError:
+            pass # If no foreign key on column, OK because it checks first
+
+        return func(self, table_name, column_name, *args, **opts)
+    return _column_rm
+
 class DatabaseOperations(generic.DatabaseOperations):
-
     """
     MySQL implementation of database operations.
-    
+
     MySQL has no DDL transaction support This can confuse people when they ask
     how to roll back - hence the dry runs, etc., found in the migration code.
     """
-    
+
     backend_name = "mysql"
     alter_string_set_type = ''
     alter_string_set_null = 'MODIFY %(column)s %(type)s NULL;'
         self._constraint_references = {}
         self._reverse_cache = {}
         super(DatabaseOperations, self).__init__(db_alias)
- 
+
     def _is_valid_cache(self, db_name, table_name):
         cache = self._constraint_cache
         # we cache the whole db so if there are any tables table_name is valid
     def rename_column(self, table_name, old, new):
         if old == new or self.dry_run:
             return []
-        
+
         rows = [x for x in self.execute('DESCRIBE %s' % (self.quote_name(table_name),)) if x[0] == old]
-        
+
         if not rows:
             raise ValueError("No column '%s' in '%s'." % (old, table_name))
-        
+
         params = (
             self.quote_name(table_name),
             self.quote_name(old),
             rows[0][4] and "%s" or "",
             rows[0][5] or "",
         )
-        
+
         sql = 'ALTER TABLE %s CHANGE COLUMN %s %s %s %s %s %s %s;' % params
-        
+
         if rows[0][4]:
             self.execute(sql, (rows[0][4],))
         else:
             self.execute(sql)
 
+    @delete_column_constraints
     def delete_column(self, table_name, name):
-        db_name = self._get_setting('NAME')
-
-        # See if there is a foreign key on this column
-        result = 0
-        for kind, cname in self.lookup_constraint(db_name, table_name, name):
-            if kind == 'FOREIGN KEY':
-                result += 1
-                fkey_name = cname
-        if result:
-            assert result == 1 # We should only have one result, otherwise there's Issues
-            cursor = self._get_connection().cursor()
-            drop_query = "ALTER TABLE %s DROP FOREIGN KEY %s"
-            self.execute(drop_query % (self.quote_name(table_name), self.quote_name(fkey_name)))
-
         super(DatabaseOperations, self).delete_column(table_name, name)
 
     @generic.invalidate_table_constraints
         if is_geom or is_text:
             field._suppress_default = True
         return field
-    
-    
+
     def _alter_set_defaults(self, field, name, params, sqls):
         """
         MySQL does not support defaults on text or blob columns.

south/tests/db_mysql.py

         db.start_transaction()
         self._create_foreign_tables(main_table, reference_table)
         db.execute_deferred_sql()
-        db_name = db._get_setting('NAME')
         constraint = db._find_foreign_constraints(main_table, 'foreign_id')[0]
         constraint_name = 'foreign_id_refs_id_%x' % (abs(hash((main_table,
             reference_table))))
         db.start_transaction()
         self._create_foreign_tables(main_table, reference_table)
         db.execute_deferred_sql()
-        db_name = db._get_setting('NAME')
         inverse = db._lookup_reverse_constraint(reference_table, 'id')
         # Hard to extract single value from set, .pop affects cache
         (cname, rev_table, rev_column) = tuple(inverse)[0]
         db.delete_table(main_table)
         db.delete_table(reference_table)
 
+    def test_delete_fk_column(self):
+        main_table = 'test_drop_foreign'
+        ref_table = 'test_df_ref'
+        self._create_foreign_tables(main_table, ref_table)
+        db.execute_deferred_sql()
+        constraints = db._find_foreign_constraints(main_table, 'foreign_id')
+        self.assertEquals(len(constraints), 1)
+        db.delete_column(main_table, 'foreign_id')
+        constraints = db._find_foreign_constraints(main_table, 'foreign_id')
+        self.assertEquals(len(constraints), 0)
+
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.