-
Notifications
You must be signed in to change notification settings - Fork 270
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
Problem when iterating if default value is '""' on postgresql-9.3 #70
Conversation
postgres@florent:/home/florent/$ psql my_db my_db=# update my_app_mymodel set field_data = '""'::json; |
We should admit empty strings can be stored by default in the DB. |
There is definitely an issue with postgresql-9.3 when there is a blank=True empty value. |
I'm having the same issue here, and i'm using MySQL. I have a JSONField with a default value of '{}' and when i generate a new migration to add this field, the '{}' is not recorded on the database. And when i try to fetch the data, the empty values throws those errors. |
Hello, in my case here is the real issue: I think you should only use default column type or text column for json field. Otherwise you will encounter real problems. In PG, json column are binary, offer a lot of interesting features at a raw level, but django can't handle them for now, some raw sql generated queries are cleary incompatible with json column. For example, first DISTINCT statement will raise an ugly error and you will need to rollback your transaction manually:/. Examples of queries on json column: If you want I can send you a new PR about it. |
@toopy I think improved JSON support for PG is being worked on: https://www.kickstarter.com/projects/mjtamlyn/improved-postgresql-support-in-django If this can wait until 1.8 or 1.9 we can see if that improves things before doing the backwards incompatible change (json column -> text column). Thoughts? |
Yes, the kickstarter project is a good thing. but imo it's bad to wait for at the opposite, we enlight the update to do. I mean, remove the pg-9.3 Otherwise, it means we need to fork the django-jsonfield or handle our json
|
@toopy I agree it's not a great solution, but the alternative is to make a pretty huge backwards incompatible change. To do that we'd need a pretty good plan on how to migrate your data. Since you'd be doing this anyway, do you want to investigate this? What exactly are the issues with switching back from JSON column type to TEXT? If we can outline the main issues, I'd be ok making this change in the next major update. |
@bradjasper Finding work around and wait better pg support is not an option. On another hand ensure a good support here, why not, but it s not an easy task. Here is a simple project we can iterate on to check everything works properly. https://github.com/toopy/mysite-jsonfield The dummy.py illustrate some errors, It looks easy not to do theses queries I agree. But when you a big app, a lot of tables, relations, entries .. etc. even if you have a good coverage, it looks very risky to move from 9.1 (our case) to 9.3 on that basis. At the opposite move from limited json column to 'basic' text column should change nothing (I you don't already wrote raw json sql queries;). (note for me: adds unittest to replace the dummy module). |
I've got a forked version that makes this change: https://github.com/bradjasper/django-jsonfield/tree/postgresql This will be kept up-to-date with master and merged in on the next major release (likely a few weeks ago). Suggestions on how to minimize pain are welcome—feel free to reopen issue or create a new one. |
In [1]: from my_app.models import MyModel
In [2]: [fo for fo in MyModel.objects.all()]
ValidationError Traceback (most recent call last)
in ()
----> 1 [fo for fo in MyModel.objects.all()]
/home/florent/.virtualenvs//local/lib/python2.7/site-packages/django/db/models/query.pyc in iter(self)
94 - Responsible for turning the rows into model objects.
95 """
---> 96 self._fetch_all()
97 return iter(self._result_cache)
98
/home/florent/.virtualenvs//local/lib/python2.7/site-packages/django/db/models/query.pyc in _fetch_all(self)
852 def _fetch_all(self):
853 if self._result_cache is None:
--> 854 self._result_cache = list(self.iterator())
855 if self._prefetch_related_lookups and not self._prefetch_done:
856 self._prefetch_related_objects()
/home/florent/.virtualenvs//local/lib/python2.7/site-packages/django/db/models/query.pyc in iterator(self)
228 obj = model_cls(*_dict(zip(init_list, row_data)))
229 else:
--> 230 obj = model(_row_data)
231
232 # Store the source database of the object
/home/florent/.virtualenvs//local/lib/python2.7/site-packages/django/db/models/base.pyc in init(self, _args, *_kwargs)
345 # without changing the logic.
346 for val, field in zip(args, fields_iter):
--> 347 setattr(self, field.attname, val)
348 else:
349 # Slower, kwargs-ready version.
/home/florent/Work//django-jsonfield/jsonfield/subclassing.pyc in set(self, obj, value)
39 # we can definitively tell if a value has already been deserialized
40 # More: #33
---> 41 obj.dict[self.field.name] = self.field.pre_init(value, obj)
42
43
/home/florent/Work//django-jsonfield/jsonfield/fields.pyc in pre_init(self, value, obj)
75 return json.loads(value, **self.load_kwargs)
76 except ValueError:
---> 77 raise ValidationError(_("Enter valid JSON"))
78
79 return value
ValidationError: [u'Enter valid JSON']