Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#22581 closed Bug (fixed)

Field.get_default() causes migration to fail if non-string value returned

Reported by: martin.tirsel@… 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):

https://bitbucket.org/schinckel/django-jsonfield/src/28c51eb06a65c1e7b5d8022031aebb034e0c129c/jsonfield/fields.py?at=default#cl-58

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 loic84, 11 years ago

Cc: loic84 and added
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug
Version: 1.7-beta-2master

comment:2 by Andrew Godwin, 11 years ago

I suspect the fix for this is to route the default values through get_prep_value on the field.

comment:3 by Andrew Godwin, 11 years ago

Owner: changed from nobody to Andrew Godwin
Status: newassigned

comment:4 by loic84, 11 years ago

@andrewgodwin +1 for get_prep_value.

comment:5 by Anssi Kääriäinen, 11 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 Andrew Godwin, 11 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 Anssi Kääriäinen, 11 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 loic84, 11 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 Andrew Godwin <andrew@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In e9a456d11b5b63389b466ed435ef3915f0e79e4c:

Fixed #22581: Pass default values for schema through get_db_prep_save()

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

In d8bf415ab29ef3ef843286e69c646ebcce58c172:

[1.7.x] Fixed #22581: Pass default values for schema through get_db_prep_save()

comment:11 by Andrew Godwin, 11 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).

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