Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSONField default passed to SQL create column query #312

Closed
ConorMcGee opened this issue Jul 28, 2016 · 11 comments
Closed

JSONField default passed to SQL create column query #312

ConorMcGee opened this issue Jul 28, 2016 · 11 comments
Assignees

Comments

@ConorMcGee
Copy link
Contributor

Summary: Migrations created to add a JSONField include default, causing query to fail.

Django Version: Django 1.9.2-8

Database and version used: MySQL 5.7.11

Version: Django-MySQL 1.0.1

Hi,
I've been having some inconsistent behaviour with this so sorry if this is confusing. In the current situation I'm stuck in, I have model that was created with a JSONField without any problems, and I'm trying to add another JSONField. It keeps failing because the migrations tries to set a default on the field, which MySQL doesn't allow:
(django.db.utils.OperationalError: (1101, "BLOB, TEXT, GEOMETRY or JSON column 'tables' can't have a default value"))
I think I know why this is happening: https://github.com/django/django/blob/6bf7964023487f2a352084e74aca27aecb354d6c/django/db/backends/mysql/schema.py#L28
That method tells migrations to leave out the default for certain fields, and I believe JSONField would now need to be included among those, as the django_mysql JSONField implicitly sets dict as its default.

I can't immediately see how this could be rectified here. Do you think it requires a bug report for Django? Or maybe I've overlooked something.

I know I've left out full tracebacks and stuff but that's just because I'm pretty sure I've located the issue above. Let me know if you'd like more.

@adamchainz
Copy link
Owner

Yes that would need fixing at the Django level. You should file a ticket there and link here.

As a workaround you should be able to remove the migration operations on the existing field if you remove the migration operations on it from the auto-generated migration (why is it detected if changed if it's a separate field you're adding❓ ).

@adamchainz
Copy link
Owner

Also send me the django ticket link, I might be able to help on it :)

@ConorMcGee
Copy link
Contributor Author

Sorry to be clear it's an AddField migration that's failing. I actually got around it for now by monkey-patching the method in the migration. I'll check back in when I get a chance to file the bug. Thanks!

@adamchainz
Copy link
Owner

It's the AddField on the new field that fails? 😢

@adamchainz adamchainz self-assigned this Jul 28, 2016
@ConorMcGee
Copy link
Contributor Author

Yes AddField in this instance. Ticket created here.

@adamchainz
Copy link
Owner

Ok I've made a PR against Django at django/django#6997, closing this since that will resolve it. If you need it before Django < 1.11 it's possible to fix it with your own database backend by subclassing the existing one, its SchemaEditor class, and including the fixed skip_default method. I can help you with this if you need it!

@ConorMcGee
Copy link
Contributor Author

Great! Thanks very much. I've already overridden the method for now, as you said.

@geeknam
Copy link

geeknam commented Mar 23, 2017

@adamchainz just wondering if there's an easy way to monkey patch skip_default? If not, would you be able to provide a snippet on how you subclass the SchemaEditor?

@adamchainz
Copy link
Owner

Yes you could monkey patch, or better try patchy.patch and create a patch file from the difference between the two: https://github.com/adamchainz/patchy

IT's more than a snippet to subclass the database backend since you need to copy a few files. Here's an example of another one I have done: https://github.com/YPlan/django-perf-rec/tree/master/tests/django18_sqlite3_backend

HTH

@iosifnicolae2
Copy link

You can set Default value to None:
image

@adamchainz
Copy link
Owner

@iosifnicolae2 or upgrade your Django :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants