#22581 closed Bug (fixed)
Field.get_default() causes migration to fail if non-string value returned
| Reported by: | Owned by: | Andrew Godwin | |
|---|---|---|---|
| Component: | Migrations | Version: | dev |
| Severity: | Release blocker | Keywords: | migration |
| Cc: | loic84, and | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
If get_default() method on a custom field returns for example a dictionary instance, then the migration fails. Here is an example of such implementation (djano-jsonfield):
I created a custom user, where I wanted to add:
class User(AbstractBaseUser, PermissionsMixin): # ... data = JSONField(default='{}') # ...
$ ./manage.py makemigrations
Migrations for 'account':
0002_user_data.py:
- Add field data to user
$ ./manage.py migrate
Operations to perform:
Synchronize unmigrated apps: admin, contenttypes, auth, sessions
Apply all migrations: account
Synchronizing apps without migrations:
Creating tables...
Installing custom SQL...
Installing indexes...
Running migrations:
Applying account.0002_user_data...Traceback (most recent call last):
File "./manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 427, in execute_from_command_line
utility.execute()
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 419, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/core/management/base.py", line 337, in execute
output = self.handle(*args, **options)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 146, in handle
executor.migrate(targets, plan, fake=options.get("fake", False))
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 60, in migrate
self.apply_migration(migration, fake=fake)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 94, in apply_migration
migration.apply(project_state, schema_editor)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/migrations/migration.py", line 97, in apply
operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 36, in database_forwards
field,
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/backends/schema.py", line 395, in add_field
self.execute(sql, params)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/backends/schema.py", line 98, in execute
cursor.execute(sql, params)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 81, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
return self.cursor.execute(sql, params)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/home/bruce/.virtualenv/paddle/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: can't adapt type 'dict'
Variables state:
params: [{}]
sql: u'ALTER TABLE "account_user" ADD COLUMN "data" text DEFAULT %s NOT NULL'
I pip installed Django from stable/1.7.x branch and using psycopg2==2.5.2 The get_default() method can return anything (as described here - https://code.djangoproject.com/ticket/8633#comment:2), so Django should handle handle such cases somehow.
Change History (11)
comment:1 by , 12 years ago
| Cc: | added |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
| Version: | 1.7-beta-2 → master |
comment:2 by , 12 years ago
comment:3 by , 12 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 12 years ago
Isn't the correct method to call get_db_prep_value()? Looking at DateTimeField's implementation it seems get_prep_value() might not be enough.
comment:6 by , 12 years ago
Well, the difference is that we're not going to database representation (SchemaEditor will handle that), we're just going to a serializable representation, so get_prep_value should be enough. I'll know more when I implement this and some tests tomorrow :)
comment:7 by , 12 years ago
Then maybe value_to_string() is better than get_db_prep_value(). The method is intended for serializing field values, so it seems to fit the use case here perfectly.
comment:8 by , 12 years ago
@akaariai, when a NOT NULL field is added, it's given a one-off default at the db level. If we used value_to_string() wouldn't that cast things like int or float into strings?
comment:9 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:11 by , 12 years ago
Turns out that akaariai was pretty correct; this was database-level directly, though it turns out this matches get_db_prep_save better (as that's for values that are saved rather than for queries, so it'll match better).
I suspect the fix for this is to route the default values through get_prep_value on the field.