Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#21783 closed Bug (fixed)

Adding a non-nullable field fails on SQLite

Reported by: Florian Apolloner Owned by: Andrew Godwin <andrew@…>
Component: Migrations Version: dev
Severity: Release blocker Keywords:
Cc: Andrew Godwin, info@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

as the title says; just add a new field to a model and provide a default for existing rows; will fail during migration with:

Operations to perform:
  Synchronize unmigrated apps: admin, contenttypes, auth, sessions
  Apply all migrations: oversight
Synchronizing apps without migrations:
  Creating tables...
  Installing custom SQL...
  Installing indexes...
Installed 0 object(s) from 0 fixture(s)
Running migrations:
  Applying oversight.0003_sensor_unit...Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/florian/sources/django.git/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/home/florian/sources/django.git/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/florian/sources/django.git/django/core/management/base.py", line 244, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/florian/sources/django.git/django/core/management/base.py", line 291, in execute
    output = self.handle(*args, **options)
  File "/home/florian/sources/django.git/django/core/management/commands/migrate.py", line 145, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/home/florian/sources/django.git/django/db/migrations/executor.py", line 60, in migrate
    self.apply_migration(migration, fake=fake)
  File "/home/florian/sources/django.git/django/db/migrations/executor.py", line 94, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/home/florian/sources/django.git/django/db/migrations/migration.py", line 97, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/home/florian/sources/django.git/django/db/migrations/operations/fields.py", line 30, in database_forwards
    schema_editor.add_field(from_model, to_model._meta.get_field_by_name(self.name)[0])
  File "/home/florian/sources/django.git/django/db/backends/sqlite3/schema.py", line 91, in add_field
    self._remake_table(model, create_fields=[field])
  File "/home/florian/sources/django.git/django/db/backends/sqlite3/schema.py", line 63, in _remake_table
    self.quote_name(model._meta.db_table),
  File "/home/florian/sources/django.git/django/db/backends/schema.py", line 96, in execute
    cursor.execute(sql, params)
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 77, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
  File "/home/florian/sources/django.git/django/db/utils.py", line 93, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
  File "/home/florian/sources/django.git/django/db/backends/sqlite3/base.py", line 489, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.IntegrityError: NOT NULL constraint failed: oversight_sensor__new.unit

Change History (8)

comment:1 by Andrew Godwin <andrew@…>, 11 years ago

Owner: set to Andrew Godwin <andrew@…>
Resolution: fixed
Status: newclosed

In e802c97581594e3a37e6497443b105ecb9920a55:

Fixed #21783: Use defaults for adding NOT NULL on sqlite

comment:2 by Andrew Godwin <andrew@…>, 11 years ago

In af4a8478e4beaa5c870620d6b5a2555eaed8b850:

Fixed #21783: (again) Found second source of bug, also squashed it.

comment:3 by Markus Holtermann, 11 years ago

Cc: info@… added
Resolution: fixed
Status: closednew

If you have a BooleanField with default=False this still fails, 'cause the check for get_default() returns False. Using has_default() solves this problem.

However, there is another issue that comes up:

If default=True the query to copy the existing data is:

INSERT INTO "boolfield_foo__new" (bar, id) SELECT True, id FROM "boolfield_foo"

This results in an

django.db.utils.OperationalError: no such column: True

(same goes for default=False respectively) which can be fixed by moving the isinstance check for boolean values before the integers in https://github.com/django/django/blob/e802c97581594e3a37e6497443b105ecb9920a55/django/db/backends/sqlite3/base.py#L218-L235

Bottom line the patch is

diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py
index cf0326d..3c8f170 100644
--- a/django/db/backends/sqlite3/base.py
+++ b/django/db/backends/sqlite3/base.py
@@ -223,12 +223,12 @@ class DatabaseOperations(BaseDatabaseOperations):
         except _sqlite3.ProgrammingError:
             pass
         # Manual emulation of SQLite parameter quoting
-        if isinstance(value, six.integer_types):
+        if isinstance(value, type(True)):
+            return str(int(value))
+        elif isinstance(value, six.integer_types):
             return str(value)
         elif isinstance(value, six.string_types):
             return '"%s"' % six.text_type(value)
-        elif isinstance(value, type(True)):
-            return str(int(value))
         elif value is None:
             return "NULL"
         else:
diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py
index 1ad5bfc..cd80d45 100644
--- a/django/db/backends/sqlite3/schema.py
+++ b/django/db/backends/sqlite3/schema.py
@@ -30,7 +30,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
         for field in create_fields:
             body[field.name] = field
             # If there's a default, insert it into the copy map
-            if field.get_default():
+            if field.has_default():
                 mapping[field.column] = self.connection.ops.quote_parameter(
                     field.get_default()
                 )

Thanks bmispelon for debugging.

comment:4 by Markus Holtermann, 11 years ago

Has patch: set
Needs tests: set

comment:5 by Andrew Godwin <andrew@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 2a30b39f40265c9228fc2e706bcb6e1df1aae991:

Fixed #21783: More SQLite default fun with nulls.

comment:6 by Samuel Spencer, 10 years ago

Call me dense, but I'm having trouble figuring out which version of Django this was integrated into.

comment:7 by Florian Apolloner, 10 years ago

If you click on the commit link, github will show you on which branches this was committed too. in this case 1.7 and 1.8

comment:8 by Markus Holtermann, 10 years ago

Django 1.7. So, the commit is part of the initial release of Django migrations.

Note: See TracTickets for help on using tickets.
Back to Top