Opened 2 years ago

Closed 2 years ago

Last modified 15 months ago

#21783 closed Bug (fixed)

Adding a non-nullable field fails on SQLite

Reported by: apollo13 Owned by: Andrew Godwin <andrew@…>
Component: Migrations Version: master
Severity: Release blocker Keywords:
Cc: andrewgodwin, 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 Changed 2 years ago by Andrew Godwin <andrew@…>

  • Owner set to Andrew Godwin <andrew@…>
  • Resolution set to fixed
  • Status changed from new to closed

In e802c97581594e3a37e6497443b105ecb9920a55:

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

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

In af4a8478e4beaa5c870620d6b5a2555eaed8b850:

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

comment:3 Changed 2 years ago by MarkusH

  • Cc info@… added
  • Resolution fixed deleted
  • Status changed from closed to new

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 Changed 2 years ago by MarkusH

  • Has patch set
  • Needs tests set

comment:5 Changed 2 years ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 2a30b39f40265c9228fc2e706bcb6e1df1aae991:

Fixed #21783: More SQLite default fun with nulls.

comment:6 Changed 15 months ago by LegoStormtroopr

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

comment:7 Changed 15 months ago by apollo13

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 Changed 15 months ago by MarkusH

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