Opened 3 years ago
Last modified 7 weeks ago
#33548 assigned Cleanup/optimization
Use -> operator to implement KeyTransform on SQLite 3.38+
Reported by: | Sage Abdullah | Owned by: | Anže Pečar |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | sqlite jsonfield keytransform |
Cc: | bcail, rajdesai24, Paolo Melchiorre | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
SQLite added support for ->
and ->>
operators as of version 3.38.0. From the docs:
Both the
->
and->>
operators select the same subcomponent of the JSON to their left. The difference is that->
always returns a JSON representation of that subcomponent and the->>
operator always returns an SQL representation of that subcomponent. Thus, these operators are subtly different from a two-argumentjson_extract()
function call. A call tojson_extract()
with two arguments will return a JSON representation of the subcomponent if and only if the subcomponent is a JSON array or object, and will return an SQL representation of the subcomponent if the subcomponent is a JSONnull
, string, or numeric value.
It would be nice to have this as a cleanup/optimization, e.g. for the workaround introduced in this PR. Still, we need to check whether the SQLite bundled in the user's Python installation supports it though, so we can't just replace the current implementation.
Change History (14)
comment:1 by , 3 years ago
Summary: | Use -> operator to implement KeyTransform on SQLite >= 3.38.0 → Use -> operator to implement KeyTransform on SQLite 3.38+ |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 2 years ago
Cc: | added |
---|
For what it's worth, the SQLite tests pass on SQLite 3.39.4 if you replace JSON_EXTRACT with ->>, like this:
diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py index 7296fe42bc..accc6ae61a 100644 --- a/django/db/models/fields/json.py +++ b/django/db/models/fields/json.py @@ -361,7 +361,7 @@ class KeyTransform(Transform): ) return ( "(CASE WHEN JSON_TYPE(%s, %%s) IN (%s) " - "THEN JSON_TYPE(%s, %%s) ELSE JSON_EXTRACT(%s, %%s) END)" + "THEN JSON_TYPE(%s, %%s) ELSE (%s ->> %%s) END)" ) % (lhs, datatype_values, lhs, lhs), (tuple(params) + (json_path,)) * 3
comment:3 by , 20 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 19 months ago
I am quite new to this topic but based on what I can understand how about we put a simple version check and return the value accordingly
something like the following which checks the version and returns the value accordingly.
if connection.sqlite_version >= '3.38': return ( "(CASE WHEN JSON_TYPE(%s, %%s) IN (%s) " "THEN JSON_TYPE(%s, %%s) ELSE (%s ->> %%s) END)" ) % (lhs, datatype_values, lhs, lhs), (tuple(params) + (json_path,)) * 3 else: return ( "(CASE WHEN JSON_TYPE(%s, %%s) IN (%s) " "THEN JSON_TYPE(%s, %%s) ELSE JSON_EXTRACT(%s, %%s) END)" ) % (lhs, datatype_values, lhs, lhs), (tuple(params) + (json_path,)) * 3
This might make it redundant but please let me know of any suggestions. Till then I will dive deeper
comment:5 by , 19 months ago
Cc: | added |
---|
comment:8 by , 6 months ago
Owner: | removed |
---|---|
Status: | assigned → new |
13 months after an assignee last commented, I guess we can assume it is open to anyone willing to work on it.
comment:9 by , 3 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
I have found this ticket during DjangoCon Europe sprints and opened a PR for it: https://github.com/django/django/pull/18241
I didn't add the version check discussed above because Python 3.10.9 updated SQLite to 3.39 so the operator should be supported on all Python versions that Django currently supports.
comment:10 by , 3 months ago
As we discussed during the DjangoCon Europe 2024 sprints, we can't be sure of the SQLite version in some cases, so a version check can be useful.
comment:11 by , 3 months ago
I have added the version check to my PR, thank you Paolo, for the quick PR review!
comment:13 by , 3 months ago
Has patch: | set |
---|
comment:14 by , 7 weeks ago
Patch needs improvement: | set |
---|
Thanks for the ticket. Tentatively accepted, it can be tricky to implement.