[Django] #32213: QuerySet .values()/.values_list() on KeyTransforms return wrong values for double-quoted strings on SQLite and Oracle
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: assigned
Component: Database | Version: 3.1
layer (models, ORM) |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Carlton Gibson (added)
* keywords: => sqlite oracle json
* severity: Release blocker => Normal
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:1>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: sage => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:2>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
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"))`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:3>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: bcail (added)
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:4>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:4 bcail]:
> This test passes on both SQLite and Postgresql for me. What do I need to
change to trigger the wrong behavior?
Add the second object `obj2 =
NullableJSONModel.objects.create(value={"key": "value"})`, `obj2` should
not be returned when you filter against `value__key='"value"'`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:5>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:4 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"']
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:5>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by bcail):
Thanks! That works for me, to show the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:6>
Django
double-quoted strings on SQLite and Oracle
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by bcail):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:7>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:8>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I have submitted a Pull Request with the fix and regression tests:
https://github.com/django/django/pull/20487
The fix addresses the issue where SQLite `KeyTransform` lookups returned
unquoted raw strings, causing incorrect `json.loads` parsing.
Verification:
1. Added a regression test `test_ticket_32213` covering quoted strings and
numeric strings on SQLite.
2. Ran the full `model_fields.test_jsonfield` suite (124 tests), which
passed successfully, ensuring no regressions in existing JSON
functionality.
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:9>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => VIZZARD-X
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:10>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Left some comments for improvements on the PR mainly
1. The solution should use `KeyTransform.select_format` and not override
`JSONField.from_db_value` with `KeyTransform` specific logic
2. Existing testing setup (models, test classes) should be used instead of
a per-ticket model and test module and test case class
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:11>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite oracle json | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
I've updated the pull request to use `KeyTransform.select_format` as
suggested, ensuring the fix is isolated to the SQLite backend without
modifying `JSONField.from_db_value`.
Also added regression tests in `tests/model_fields/test_jsonfield.py`
rather than using per-ticket model which was the case previously
--
Ticket URL: <https://code.djangoproject.com/ticket/32213#comment:12>