Opened 4 years ago
Last modified 3 weeks ago
#32491 assigned Bug
Updating a field based on a JSONField's sub-value adds extra quotes [postgres]
Reported by: | Baptiste Mispelon | Owned by: | YashRaj1506 |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Giannis Terzopoulos | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Consider the following model:
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:
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
Change History (7)
comment:1 by , 4 years ago
comment:2 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Bonjour Baptiste, always a pleasure to see you around :)
Initially, I thought that adding an explicit Cast (using update(name=Cast(F('dataname'), 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('dataname')) 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 remove the lookups hacks for textual and numeric values as right now it's not possible to go things like jsonfield__name__gt="Simon"
comment:3 by , 4 years ago
Also FYI: There is an open ticket about documenting KeyTransform()
and KeyTextTransform()
: #26511
comment:4 by , 14 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 14 months ago
Owner: | removed |
---|---|
Status: | assigned → new |
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
comment:6 by , 9 months ago
Cc: | added |
---|
comment:7 by , 3 weeks ago
Owner: | set to |
---|---|
Status: | new → assigned |
Initially, I thought that adding an explicit
Cast
(usingupdate(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 quickgit 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 yieldsdjango
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
overKeyTransform
) 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.