#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 , 11 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
Version: | 1.7-beta-2 → master |
comment:2 by , 11 years ago
comment:3 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 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 , 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 , 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 , 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 , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:11 by , 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).
I suspect the fix for this is to route the default values through get_prep_value on the field.