shaib avatar shaib committed 26cd2d2 Merge

Merge the dont-leave-defaults work

Comments (0)

Files changed (5)


             sqls.append(('ALTER COLUMN %s DROP DEFAULT' % (self.quote_name(name),), []))
+    def _update_nulls_to_default(self, params, field):
+        "Subcommand of alter_column that updates nulls to default value (overrideable)"
+        default = field.get_default()
+        self.execute('UPDATE %(table_name)s SET %(column)s=%%s WHERE %(column)s IS NULL' % params, [default])
     def alter_column(self, table_name, name, field, explicit_name=True, ignore_constraints=False):
         params = {
             "column": self.quote_name(name),
             "type": self._db_type_for_alter_column(field),
-            "table_name": table_name
+            "table_name": self.quote_name(table_name)
         # SQLs is a list of (SQL, values) pairs.
         # Add any field- and backend- specific modifications
         self._alter_add_column_mods(field, name, params, sqls)
         # Next, nullity
-        if field.null:
+        if field.null or field.has_default():
             sqls.append((self.alter_string_set_null % params, []))
             sqls.append((self.alter_string_drop_null % params, []))
-        # Next, set any default
-        self._alter_set_defaults(field, name, params, sqls)
-        # Finally, actually change the column
+        # Actually change the column (step 1 -- Nullity may need to be fixed)
         if self.allows_combined_alters:
             sqls, values = zip(*sqls)
             # Databases like e.g. MySQL don't like more than one alter at once.
             for sql, values in sqls:
                 self.execute("ALTER TABLE %s %s;" % (self.quote_name(table_name), sql), values)
+        if not field.null and field.has_default():
+            # Final fixes
+            self._update_nulls_to_default(params, field)
+            self.execute("ALTER TABLE %s %s;" % (self.quote_name(table_name), self.alter_string_drop_null % params), [])            
         if not ignore_constraints:
             # Add back FK constraints if needed
         Generates a full SQL statement to add a foreign key constraint
-        constraint_name = '%s_refs_%s_%x' % (from_column_name, to_column_name, self._digest(from_table_name, to_table_name))
+        constraint_name = '%s_refs_%s_%s' % (from_column_name, to_column_name, self._digest(from_table_name, to_table_name))
         return 'ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s)%s;' % (


     alter_string_set_type =     'ALTER TABLE %(table_name)s MODIFY %(column)s %(type)s %(nullity)s;'
     alter_string_set_default =  'ALTER TABLE %(table_name)s MODIFY %(column)s DEFAULT %(default)s;'
+    alter_string_update_nulls_to_default = \
+                                'UPDATE %(table_name)s SET %(column)s = %(default)s WHERE %(column)s IS NULL;'
     add_column_string =         'ALTER TABLE %s ADD %s;'
     delete_column_string =      'ALTER TABLE %s DROP COLUMN %s;'
     add_constraint_string =     'ALTER TABLE %(table_name)s ADD CONSTRAINT %(constraint)s %(clause)s'
         columns = []
         autoinc_sql = ''
         for field_name, field in fields:
+            # avoid default values in CREATE TABLE statements (#925)
+            field._suppress_default = True
             col = self.column_sql(table_name, field_name, field)
             if not col:
         if field.null:
             params['nullity'] = 'NULL'
-        if not field.null and field.has_default():
-            params['default'] = self._default_value_workaround(field.get_default())
         sql_templates = [
             (self.alter_string_set_type, params),
-            (self.alter_string_set_default, params.copy()),
+            (self.alter_string_set_default, params),
+        if not field.null and field.has_default():
+            # Use default for rows that had nulls. To support the case where
+            # the new default does not fit the old type, we need to first change
+            # the column type to the new type, but null=True; then set the default;
+            # then complete the type change. 
+            def change_params(**kw):
+                "A little helper for non-destructively changing the params"
+                p = params.copy()
+                p.update(kw)
+                return p
+            sql_templates[:0] = [
+                (self.alter_string_set_type, change_params(nullity='NULL')),
+                (self.alter_string_update_nulls_to_default, change_params(default=self._default_value_workaround(field.get_default()))),
+            ]
         # drop CHECK constraints. Make sure this is executed before the ALTER TABLE statements
         # generated above, since those statements recreate the constraints we delete here.
-    def add_column(self, table_name, name, field, keep_default=True):
+    def add_column(self, table_name, name, field, keep_default=False):
         sql = self.column_sql(table_name, name, field)
         sql = self.adj_column_sql(sql)


     # 1) The sql-server-specific call to _fix_field_definition
     # 2) Removing a default, when needed, by calling drop_default and not the more general alter_column
-    def add_column(self, table_name, name, field, keep_default=True):
+    def add_column(self, table_name, name, field, keep_default=False):
         Adds the column 'name' to the table 'table_name'.
         Uses the 'field' paramater, a django.db.models.fields.Field instance,
         # Run
-        generic.DatabaseOperations.create_table(self, table_name, field_defs)
+        super(DatabaseOperations, self).create_table(table_name, field_defs)
     def _find_referencing_fks(self, table_name):
         "MSSQL does not support cascading FKs when dropping tables, we need to implement."


         self._remake_table(table_name, added={
             field.column: self._column_sql_for_create(table_name, name, field, False),
+        # Now, remove any defaults
+        field._suppress_default = True
+        self.alter_column(table_name, name, field)
     def _get_full_table_description(self, connection, cursor, table_name):
         cursor.execute('PRAGMA table_info(%s)' % connection.ops.quote_name(table_name))
         The argument is accepted for API compatibility with the generic
         DatabaseOperations.alter_column() method.
+        # Change nulls to default if needed
+        if not field.null and field.has_default():
+            params = {
+                "column": self.quote_name(name),
+                "table_name": self.quote_name(table_name)
+            }            
+            self._update_nulls_to_default(params, field)
         # Remake the table correctly
+        field._suppress_default = True
         self._remake_table(table_name, altered={
             name: self._column_sql_for_create(table_name, name, field, explicit_name),


   "Non-existent table could be selected!")
+    def test_create_default(self):
+        """
+        Test creation of tables, make sure defaults are not left in the database
+        """
+        db.create_table("test_create_default", [('a', models.IntegerField()),
+                                                ('b', models.IntegerField(default=17))])
+        cursor = connection.cursor()
+        self.assertRaises(IntegrityError, cursor.execute, "INSERT INTO test_create_default(a) VALUES (17)")
     def test_delete(self):
         Test deletion of tables.
         # insert some data so we can test the default values of the added column
         db.execute("INSERT INTO test_addnbc (spam, eggs) VALUES (%s, 1)", [False])
         # try selecting from the new columns to make sure they were properly created
-        false, null, true = db.execute("SELECT spam,add1,add2 FROM test_addnbc")[0][0:3]
-        self.assertTrue(true)
-        self.assertEquals(null, None)
+        false, null1, null2 = db.execute("SELECT spam,add1,add2 FROM test_addnbc")[0][0:3]
+        self.assertIsNone(null1, "Null boolean field with no value inserted returns non-null")
+        self.assertIsNone(null2, "Null boolean field (added with default) with no value inserted returns non-null")
         self.assertEquals(false, False)
             ('eggs', models.IntegerField()),
         # Change spam default
-        db.alter_column("test_altercd", "spam", models.CharField(max_length=30, default="loof"))
+        db.alter_column("test_altercd", "spam", models.CharField(max_length=30, default="loof", null=True))
+        # Assert the default is not in the database
+        db.execute("INSERT INTO test_altercd (eggs) values (12)")
+        null = db.execute("SELECT spam FROM test_altercd")[0][0]
+        self.assertFalse(null, "Default for char field was installed into database")
     def test_mysql_defaults(self):
     def test_datetime_default(self):
-        Test that defaults are created correctly for datetime columns
+        Test that defaults are correctly not created for datetime columns
         end_of_world = datetime.datetime(2012, 12, 21, 0, 0, 1)
             ('col2', models.DateTimeField(null=True)),
+        # insert a row
+        db.execute("INSERT INTO test_datetime_def (col0, col1, col2) values (null,%s,null)", [end_of_world])
         db.alter_column("test_datetime_def", "col2", models.DateTimeField(default=end_of_world))
         db.add_column("test_datetime_def", "col3", models.DateTimeField(default=end_of_world))
-        # There should not be a default in the database for col1
+        # In the single existing row, we now expect col1=col2=col3=end_of_world...
-        self.assertRaises(
-            IntegrityError,
-            db.execute, "insert into test_datetime_def (col0) values (null)"
-        )
-        db.rollback_transaction()
-        db.start_transaction()
-        # There should be for the others
-        db.execute("insert into test_datetime_def (col0, col1) values (null, %s)", [end_of_world])
         ends = db.execute("select col1,col2,col3 from test_datetime_def")[0]
         self.failUnlessEqual(len(ends), 3)
         for e in ends:
             self.failUnlessEqual(e, end_of_world)
+        db.commit_transaction()
+        # ...but there should not be a default in the database for any column (other than the 'null' for col0)
+        for cols in ["col1,col2", "col1,col3","col2,col3"]:
+            db.start_transaction()
+            statement = "insert into test_datetime_def (col0,%s) values (null,%%s,%%s)" % cols
+            self.assertRaises(
+                IntegrityError,
+                db.execute, statement, [end_of_world, end_of_world]
+            )
+            db.rollback_transaction()
+        db.start_transaction() # To preserve the sanity and semantics of this test class
     def test_add_unique_fk(self):
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
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.