[Django] #32491: Updating a field based on a JSONField's sub-value adds extra quotes [postgres]
Django
[postgres]
-------------------------------------+-------------------------------------
Reporter: Baptiste | Owner: nobody
Mispelon |
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Consider the following model:
{{{#!py
class JSONNamedModel(models.Model):
name = models.CharField(max_length=10, blank=True)
data = models.JSONField()
}}}
The following testcase seems to pass under sqlite but fails under postgres
(postgresql version 13.1, psycopg2 version 2.8.6). I haven't tested other
backends:
{{{#!py
class ReproductionTestCase(TestCase):
def test_issue_update(self):
JSONNamedModel.objects.create(data={'name': 'django'})
JSONNamedModel.objects.update(name=F('data__name'))
self.assertQuerysetEqual(
JSONNamedModel.objects.all(),
['django'],
transform=lambda m: m.name,
)
}}}
To summarize what happens in the test, if you have an instance with
`data={'name': 'django'}` and you do `update(name=F('data__name'))`, you
end up with an instance that has `name='"django"'` (the original name with
extra double quotes around it).
Interestingly, if you start with `django"`, you end up with `"django\""`
(the original `"` is backslash-escaped and the whole thing is wrapped by
double quotes).
Not sure how relevant this is, but while digging into this issue I noticed
that the test failure changed somewhat recently (bisected down to commit
8b040e3cbbb2e81420e777afc3ca48a1c8f4dd5a).
Before that commit, the error was `django.core.exceptions.FieldError:
Joined field references are not permitted in this query`
--
Ticket URL: <https://code.djangoproject.com/ticket/32491>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
[postgres]
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Baptiste Mispelon):
Initially, I thought that adding an explicit `Cast` (using
`update(name=Cast(F('data__name'), CharField()))`) might solve the issue,
but the test still fails in the same way.
I got it to work using `KeyTextTransform` (which doesn't seem to be
documented based on a quick `git grep`):
`update(name=KeyTextTransform('name', 'data'))`.
Adding to my confusion was the fact that this behavior doesn't appear when
using `annotate`: `annotate(dataname=F('data__name'))` correctly yields
`django` without the extra quotes.
I'm not sure if this is really a bug or how fixable it is (I don't see how
Django could guess that it should use `KeyTextTransform` over
`KeyTransform`) but silently adding extra `"` seems pretty bad.
Having `KeyTransform/KeyTextTransform` documented could help, though I'm
not sure if I would have discovered their existence even if they were.
--
Ticket URL: <https://code.djangoproject.com/ticket/32491#comment:1>
Django
[postgres]
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Bonjour Baptiste, always a pleasure to see you around :)
> Initially, I thought that adding an explicit Cast (using
update(name=Cast(F('data__name'), CharField()))) might solve the issue,
but the test still fails in the same way.
yeah that's expected since `::json::text` is pretty much the equivalent of
`json.dumps` (e.g. see #27257 for a similar problem)
> Adding to my confusion was the fact that this behavior doesn't appear
when using annotate: annotate(dataname=F('data__name')) correctly yields
django without the extra quotes.
Right this happens because `JSONField.from_db_value` attempts a
`json.loads` on the return value so `'"foo"' -> 'foo'`
> I'm not sure if this is really a bug or how fixable it is (I don't see
how Django could guess that it should use KeyTextTransform over
KeyTransform) but silently adding extra " seems pretty bad. Having
KeyTransform/KeyTextTransform documented could help, though I'm not sure
if I would have discovered their existence even if they were.
It could be done by having transforms used `for_save` have access to
`output_field` of the left hand side of the `UPDATE`/`INSERT` (e.g a
`CharField`) in this case and decide whether `->>` or `->` should be used
but that would be slightly backward incompatible.
An alternative could be to introduce an `__astext` lookup on `JSONField`
that would translate to the usage of `KeyTextTransform`. That would make
it explicit which SQL operator must be used and a similar approach could
be used for `__asint` and `__asfloat` to
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/db/models/fields/json.py#L462-L533
remove the lookups hacks for textual and numeric values] as right now it's
not possible to go things like `jsonfield__name__gt="Simon"`
--
Ticket URL: <https://code.djangoproject.com/ticket/32491#comment:2>
Django
[postgres]
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
Also FYI: There is an open ticket about documenting `KeyTransform()` and
`KeyTextTransform()`: #26511
--
Ticket URL: <https://code.djangoproject.com/ticket/32491#comment:3>
Django
[postgres]
-------------------------------------+-------------------------------------
| m_moeinzadeh
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => m_moeinzadeh
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32491#comment:4>
Django
[postgres]
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: m_moeinzadeh => (none)
* status: assigned => new
Comment:
neither `models.JSONField()` functions or backend converters of django
support changing the value of the `models.JSONField()`'s value
both `save()` and `get()` functions of django area working without any
bugs and the extra set of quotations is being added by while converting
the query data to flat data
in order to fix it, a new update is required on `converters` so they can
apply on the value of a `models.JSONField()` value not only on the key of
it's value
therefore this ticket requires major changes that are outside of this
tickets tritory that might cause great problems in other features and i
suggest adding full support for `models.JSONField()` to `converters`
--
Ticket URL: <https://code.djangoproject.com/ticket/32491#comment:5>
Django
[postgres]
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Giannis Terzopoulos (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/32491#comment:6>