Opened 4 years ago

Last modified 2 years ago

#32213 new Bug

QuerySet.values()/.values_list() on KeyTransforms return wrong values for double-quoted strings on SQLite and Oracle

Reported by: Sage Abdullah Owned by:
Component: Database layer (models, ORM) Version: 3.1
Severity: Normal Keywords: sqlite oracle json
Cc: Carlton Gibson, bcail Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:How to create a pull request

Description

Suppose we have a JSONField with the value {'key': '"value"'}. If .values()/.values_list() are called on a jsonfield__key lookup, they return 'value' on SQLite and Oracle. On other backends, they return '"value"', which is the correct behavior.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (7)

comment:1 by Mariusz Felisiak, 4 years ago

Cc: Carlton Gibson added
Keywords: sqlite oracle json added
Severity: Release blockerNormal
Summary: QuerySet .values()/.values_list() on KeyTransforms return wrong values for double-quoted strings on SQLite and OracleQuerySet.values()/.values_list() on KeyTransforms return wrong values for double-quoted strings on SQLite and Oracle
Triage Stage: UnreviewedAccepted

Thanks, I'm not sure about severity because it's niche I don't believe that many users (if anyone) are affected by this issue. We can bump it when evaluating a patch.

comment:2 by Mariusz Felisiak, 3 years ago

Owner: Sage Abdullah removed
Status: assignednew

comment:3 by Simon Charette, 2 years ago

I'm also not sure this is event fixable since JSONField is schema less so the ORM has not way to know it should use ->> over -> for the specified key.

I think this ticket is a good candidate for wontfix consideration once KeyTextTransform is documented (#15956) as this issue can be solved by doing values_list(KeyTextTransform("key", "json_field")).

comment:4 by bcail, 2 years ago

Cc: bcail added

I'm working on a test for this case, and having trouble reproducing it.

diff --git a/tests/model_fields/test_jsonfield.py b/tests/model_fields/test_jsonfield.py
index 2c32d8a4ea..ecc2b9d0d5 100644
--- a/tests/model_fields/test_jsonfield.py
+++ b/tests/model_fields/test_jsonfield.py
@@ -611,6 +611,11 @@ class TestQuerying(TestCase):
                     [obj],
                 )
 
+    def test_value(self):
+        obj = NullableJSONModel.objects.create(value={"key": '"value"'})
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='"value"').values_list("value", flat=True), [{'key': '"value"'}])
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(Q(value__has_key="key")).values_list("value", flat=True), [{'key': '"value"'}])
+
     @skipUnlessDBFeature("supports_json_field_contains")
     def test_contains(self):
         tests = [

This test passes on both SQLite and Postgresql for me. What do I need to change to trigger the wrong behavior?

in reply to:  4 comment:5 by Mariusz Felisiak, 2 years ago

Replying to bcail:

This test passes on both SQLite and Postgresql for me. What do I need to change to trigger the wrong behavior?

This ticket is about using key transforms in the values()/values_list():

# SQLite
>>> NullableJSONModel.objects.filter(value__key='"value"').values_list("value__key", flat=True)
['value']
# PostgreSQL
>>> NullableJSONModel.objects.filter(value__key='"value"').values_list("value__key", flat=True)
['"value"']

comment:6 by bcail, 2 years ago

Thanks! That works for me, to show the issue.

comment:7 by bcail, 2 years ago

Here are some tests that show the issues with various strings in a JSONField for sqlite (they pass on postgresql):

+    def test_str_with_quotes(self):
+        value = {"key": "test_value", "key2": '"value"'}
+        obj = NullableJSONModel.objects.create(value=value)
+
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values("value__key2"), [{"value__key2": '"value"'}])
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values_list("value__key2"), [('"value"',)])
+
+    def test_str_with_null_false_true(self):
+        value = {"key": "test_value", "key2": "null", "key3": "false", "key4": "true"}
+        obj = NullableJSONModel.objects.create(value=value)
+
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values_list("value__key2"), [('null',)])
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values("value__key3"), [{"value__key3": 'false'}])
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values("value__key4"), [{"value__key4": 'true'}])
+
+    def test_str_with_list_dict(self):
+        value = {"key": "test_value", "key2": "[]", "key3": "{}"}
+        obj = NullableJSONModel.objects.create(value=value)
+
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values_list("value__key2"), [("[]",)])
+        self.assertSequenceEqual(NullableJSONModel.objects.filter(value__key='test_value').values("value__key3"), [{"value__key3": "{}"}])

This code change gets the str_with_quotes test passing, but not the other two:

diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py
index 7296fe42bc..6277ce9935 100644
--- a/django/db/models/fields/json.py
+++ b/django/db/models/fields/json.py
@@ -82,8 +82,11 @@ class JSONField(CheckFieldDefaultMixin, Field):
             return value
         # Some backends (SQLite at least) extract non-string values in their
         # SQL datatypes.
-        if isinstance(expression, KeyTransform) and not isinstance(value, str):
-            return value
+        if connection.vendor == "sqlite" and isinstance(expression, KeyTransform):
+            if not isinstance(value, str):
+                return value
+            if value not in ['true', 'false', 'null'] and not value.startswith('[') and not value.startswith('{'):
+                return value
         try:
             return json.loads(value, cls=self.decoder)
         except json.JSONDecodeError:

I think that once sqlite 3.38.0 is in widespread use, and ticket #33548 is implemented (using -> and ->> in sqlite), that may take care of this issue.

Note: See TracTickets for help on using tickets.
Back to Top