Opened 2 years ago

Last modified 2 months ago

#33548 new Cleanup/optimization

Use -> operator to implement KeyTransform on SQLite 3.38+

Reported by: Sage Abdullah Owned by:
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: sqlite jsonfield keytransform
Cc: bcail, rajdesai24, Paolo Melchiorre Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
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-argument json_extract() function call. A call to json_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 JSON null, 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 (8)

comment:1 by Mariusz Felisiak, 2 years ago

Summary: Use -> operator to implement KeyTransform on SQLite >= 3.38.0Use -> operator to implement KeyTransform on SQLite 3.38+
Triage Stage: UnreviewedAccepted

Thanks for the ticket. Tentatively accepted, it can be tricky to implement.

comment:2 by bcail, 19 months ago

Cc: bcail 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 rajdesai24, 16 months ago

Owner: changed from nobody to rajdesai24
Status: newassigned

comment:4 by rajdesai24, 15 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 rajdesai24, 15 months ago

Cc: rajdesai24 added

comment:6 by Paolo Melchiorre, 8 months ago

Cc: Paolo Melchiorre added

This change would be an improvement. +1

comment:7 by bcail, 2 months ago

@rajdesai24, are you planning to open a PR for this?

comment:8 by Claude Paroz, 2 months ago

Owner: rajdesai24 removed
Status: assignednew

13 months after an assignee last commented, I guess we can assume it is open to anyone willing to work on it.

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