https://github.com/django/django/blob/master/django/contrib/postgres/fields/hstore.py#L38
https://github.com/django/django/blob/master/django/contrib/postgres/fields/array.py#L102
https://github.com/django/django/blob/master/django/contrib/postgres/fields/ranges.py#L47
JSONField defaults to the value, meaning if you pass `'{"object":
"value"}'` you get back `'{"object": "value"}'`. The same example in
HStoreField, you pass in `'{"object": "value"}'` you get back `{"object":
"value"}` -- makes the world of difference.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* owner: (none) => Williams Mendez
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:1>
* status: assigned => closed
* resolution: => invalid
Comment:
I don't think that change is correct because
[https://github.com/django/django/blob/16436f3751e9eec67a7b80b25c8e7d29a34be67e/tests/postgres_tests/test_json.py#L46-L50
a string is valid JSON] -- to treat it differently would break backwards
compatibility.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:2>
Comment (by Williams Mendez):
At first I thought the same, but when I try to store a String into a
postgres 'jsonb' field it crashes. Same when I try to fetch a string as a
json:
{{{
will_test=# select 'hello'::json ;
ERROR: invalid input syntax for type json
LINE 1: select 'hello'::json ;
^
DETAIL: Token "hello" is invalid.
CONTEXT: JSON data, line 1: hello
will_test=# select '{}'::json ;
json
------
{}
(1 row)
}}}
I'll do some research on that topic.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:3>
Comment (by Williams Mendez):
Yeah, it's totally right:
{{{
will_test=# select '"hello"'::json ;
json
---------
"hello"
(1 row)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:4>
Comment (by Javier Buzzi):
As i understood it was that `to_python()` is the representation of the
data in python, postgres could save it and do with it what it wants; store
it binary/urlencode it/ encrypt it for all it wants, but from a python
perspective that data to me, needs to be represented as a dict/simple type
(if it is "hello") -- i still don't understand why this is invalid.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:5>
Comment (by Tim Graham):
Can you be more clear about what the unexpected behavior is? (i.e. give a
failing test for `tests/postgres_tests/test_json.py`)
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:6>
Comment (by Javier Buzzi):
I added this (copy and paste from hstore.py)
{{{
--- a/django/contrib/postgres/fields/jsonb.py
+++ b/django/contrib/postgres/fields/jsonb.py
@@ -41,6 +41,14 @@ class JSONField(CheckFieldDefaultMixin, Field):
self.encoder = encoder
super().__init__(verbose_name, name, **kwargs)
+ def to_python(self, value):
+ if isinstance(value, str):
+ value = json.loads(value)
+ return value
+
+ def value_to_string(self, obj):
+ return json.dumps(self.value_from_object(obj))
+
def db_type(self, connection):
return 'jsonb'
}}}
ran the tests:
{{{
$ /django/tests/runtests.py postgres_tests.test_json --failfast
--settings=mysettings
Testing against Django installed in '/usr/local/lib/python3.6/site-
packages/Django-2.1.dev20180313024248-py3.6.egg/django' with up to 4
processes
Creating test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
System check identified no issues (0 silenced).
.......................................................
----------------------------------------------------------------------
Ran 55 tests in 0.118s
OK
Destroying test database for alias 'default'...
Destroying test database for alias 'default'...
Destroying test database for alias 'default'...
Destroying test database for alias 'default'...
Destroying test database for alias 'default'...
}}}
Here are all the steps on how i got it to work:
{{{
$ docker run --rm -it python:3.6.4 bash
apt-get update
git clone https://github.com/django/django.git
cd django
sed -i "s/^exit 101$/exit 0/" /usr/sbin/policy-rc.d
apt-get install -y libmemcached-dev postgresql vim postgresql-contrib
python setup.py install
pip install -rtests/requirements/py3.txt -rtests/requirements/postgres.txt
echo "SECRET_KEY = 'hellooooo'
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql_psycopg2',
'NAME': 'django',
'USER': 'django',
'PASSWORD': 'django',
'HOST': 'localhost',
'PORT': 5432,
}
}" > tests/mysettings.py
echo "local all all trust" >>
/etc/postgresql/9.4/main/pg_hba.conf
echo "diff --git a/django/contrib/postgres/fields/jsonb.py
b/django/contrib/postgres/fields/jsonb.py
index 3c27607..49b65ef 100644
--- a/django/contrib/postgres/fields/jsonb.py
+++ b/django/contrib/postgres/fields/jsonb.py
@@ -41,6 +41,14 @@ class JSONField(CheckFieldDefaultMixin, Field):
self.encoder = encoder
super().__init__(verbose_name, name, **kwargs)
+ def to_python(self, value):
+ if isinstance(value, str):
+ value = json.loads(value)
+ return value
+
+ def value_to_string(self, obj):
+ return json.dumps(self.value_from_object(obj))
+
def db_type(self, connection):
return 'jsonb'
" > mychanges.diff
patch django/contrib/postgres/fields/jsonb.py mychanges.diff
service postgresql start
echo "set the password to 'django'"
su postgres -c 'createuser --superuser django -P'
/django/tests/runtests.py postgres_tests.test_json --failfast
--settings=mysettings
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:7>
Comment (by Tim Graham):
What behavior changes as a result of the patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:8>
Comment (by Javier Buzzi):
@Tim, nothing that i can see.
We need this for Aloe, we have a string table to load data for our tests
and we rely heavily on the field's ability to turn a raw/string value to a
python value.
Example:
{{{
from django.db.models import BooleanField, CharField, NumberField
from django.contrib.postgres import fields
CharField.to_python(None, 'hello')
'hello'
IntegerField.to_python(None, '123')
123
BooleanField.to_python(None, 'True')
True
fields.HStoreField.to_python(None, '{"hello": "world"}')
{'hello': 'world'}
}}}
The only thing that breaks that pattern in the entire lib, is `JSONField`:
{{{
fields.JSONField.to_python(None, '{"hello": "world"}')
'{"hello": "world"}'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:9>
Comment (by Tim Graham):
I believe your proposal is backwards incompatible. Assuming that a string
should be parsed as a JSON dict rather than treating a string as a string
could be undesired. Your patch results in a test failure of
`postgres_tests.test_json.TestSerialization.test_dumping`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:10>
Comment (by Javier Buzzi):
Please explain, i don't see that on my side. (using the same setup from
above)
{{{
# /django/tests/runtests.py
postgres_tests.test_json.TestSerialization.test_dumping --failfast
--settings=mysettings
Testing against Django installed in '/usr/local/lib/python3.6/site-
packages/Django-2.1.dev20180320161010-py3.6.egg/django' with up to 4
processes
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.
----------------------------------------------------------------------
Ran 1 test in 0.004s
OK
Destroying test database for alias 'default'...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:11>
Comment (by Tim Graham):
In your patch, I think you missed the `JSONField.value_to_string()` that's
already defined. It overrides the one you added since it's declared below
yours. The test failure happens if you remove the existing
`value_to_string()` method in favor of yours. Anyway, if you provide a
pull request that passes all tests and includes a test for the new
behavior, I'll take a closer look to see if this is suitable.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:12>
Comment (by Javier Buzzi):
Thanks Tim.
Here it is: https://github.com/django/django/pull/9801
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:13>
Comment (by Javier Buzzi):
@Tim, can you look at the PR https://github.com/django/django/pull/9801
Any comments, concerns, and/or changes, will be appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:14>
* has_patch: 0 => 1
Old description:
> Every other field implements a to_python except for JSONField:
>
> https://github.com/django/django/blob/master/django/contrib/postgres/fields/hstore.py#L38
> https://github.com/django/django/blob/master/django/contrib/postgres/fields/array.py#L102
> https://github.com/django/django/blob/master/django/contrib/postgres/fields/ranges.py#L47
>
> JSONField defaults to the value, meaning if you pass `'{"object":
> "value"}'` you get back `'{"object": "value"}'`. The same example in
> HStoreField, you pass in `'{"object": "value"}'` you get back `{"object":
> "value"}` -- makes the world of difference.
New description:
Every other field implements a to_python except for JSONField:
https://github.com/django/django/blob/master/django/contrib/postgres/fields/hstore.py#L38
https://github.com/django/django/blob/master/django/contrib/postgres/fields/array.py#L102
https://github.com/django/django/blob/master/django/contrib/postgres/fields/ranges.py#L47
JSONField defaults to the value, meaning if you pass `'{"object":
"value"}'` you get back `'{"object": "value"}'`. The same example in
HStoreField, you pass in `'{"object": "value"}'` you get back `{"object":
"value"}` -- makes the world of difference.
PR. https://github.com/django/django/pull/9801
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:15>
Comment (by Tim Graham <timograham@…>):
In [changeset:"623139b5d1bd006eac78b375bcaf5948e695c3c6" 623139b]:
{{{
#!CommitTicketReference repository=""
revision="623139b5d1bd006eac78b375bcaf5948e695c3c6"
Refs #29147 --- Added JSONField serialization tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:16>
Comment (by Tim Graham):
The tests committed above demonstrate why this proposal is backwards
incompatible. I think if your application makes the assumption you've
articulated about your data, you'll have to add your own parsing.
--
Ticket URL: <https://code.djangoproject.com/ticket/29147#comment:17>