Fixed #32519 -- Added support for using key and path transforms in update() for JSONFields. #18489

adzshaf · 2024-08-17T14:31:41Z

Trac ticket number

Branch description

This PR introduces support for JSONField key transforms in update so we can do partial update JSONField more efficiently. Consider following example to update a JSONField.

>>> user_preferences = UserPreferences.objects.create(
...     settings={
...         "theme": "dark",
...         "notifications": True,
...         "font": {"size": 10, "name": "Arial"},
...     }
... )
>>> UserPreferences.objects.update(
...     settings__theme="light",
...     settings__font__size=20,
... )
>>> user_preferences = UserPreferences.objects.get(pk=user_preferences.pk)
>>> print(user_preferences.settings)
{'theme': 'light', 'notifications': True, 'font': {'size': 20, 'name': 'Arial'}}

You can also remove a key by providing NOT_PROVIDED to parameter value.

>>> from django.db.models import NOT_PROVIDED
>>> user_preferences = UserPreferences.objects.create(
...     settings={
...         "theme": "dark",
...         "notifications": True,
...         "font": {"size": 10, "name": "Arial"},
...     }
... )
>>> UserPreferences.objects.update(
...     settings__theme=NOT_PROVIDED,
...     settings__font__size=20,
... )
>>> user_preferences = UserPreferences.objects.get(pk=user_preferences.pk)
>>> print(user_preferences.settings)
{'notifications': True, 'font': {'size': 20, 'name': 'Arial'}}

This feature is implemented using the newly added databases functions JSONSet and JSONRemove which serves as abstraction on top of the suitable functions on each database backend.

This PR is a part of the Google Summer of Code program.

Checklist

This PR targets the main branch.
The commit message is written in past tense, mentions the ticket number, and ends with a period.
I have checked the "Has patch" ticket flag in the Trac system.
I have added or updated relevant tests.
I have added or updated relevant docs, including release notes if applicable.
I have attached screenshots in both light and dark modes for any UI changes.

laymonage

Really great work here! I haven't fully reviewed this, but could we squash the commit history, then break it down into smaller commits to make it easier for others to review?

I can see this broken down into at least these commits:

Adding JSONField handling for Cast() on SQLite, and the corresponding tests.
Extracting KeyTransform.unwrap_transforms() from KeyTransform.preprocess_lhs()
JSONSet + tests
JSONRemove + tests
Changes to UpdateQuery.add_update_values() in subqueries.py and addition of Transform.get_update_expression() in lookups.py + tests for the NotImplementedError and FieldError
Addition of KeyTransform.get_update_expression() + tests for using key transforms in .update()

Thanks!

laymonage · 2024-08-20T23:07:47Z

django/db/backends/oracle/features.py

This seems correct according to https://apex.oracle.com/database-features/

However, the function seems to be documented for 19c as well: https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/json_transform.html

According to https://stackoverflow.com/a/75286939 it was backported to 19c in 19.10. Could we update this and see if it works? According to https://code.djangoproject.com/wiki/CI we use v19.3.0.0.0 on the CI though, so that may need to be upgraded, or we can test ourselves.

We agreed on following the https://apex.oracle.com/database-features/ website.

django/db/models/fields/json.py

laymonage · 2024-08-20T23:10:36Z

django/db/models/functions/comparison.py

Could we extract the changes in Cast(), along with the corresponding test, into a separate commit please?

Done ✔️

laymonage · 2024-08-20T23:18:48Z

django/db/models/sql/subqueries.py

This logic seems similar to the one that already exists in Query.try_transform(). We've discussed this before and encountered a few issues in doing so, but I still wonder if it's possible to reuse that method. I'll do some experiments later.

laymonage · 2024-08-20T23:22:13Z

django/db/models/sql/subqueries.py

Could we extract the changes in this file and the Transform.get_update_expression() method in lookups.py, along with the tests (albeit only testing for the NotImplementedError and FieldError) into a separate commit please?

Done ✔️

laymonage · 2024-08-20T23:24:31Z

django/db/models/functions/json.py

Let's create a separate commit for JSONSet+tests, and another commit for JSONRemove+tests. Then, create a separate commit for adding KeyTransform.get_update_expression() and the corresponding tests.

Done ✔️

This allows us to mark serialized JSON data as JSON instead of a regular string when passing it to the database.

…ss_lhs().

laymonage

Thanks!

@adzshaf and I have been iterating on this together for quite a while. I have yet to fully review the tests and docs, but in general we're pretty happy with the implementation's approach.

There are bits I'm still not sure about, mostly around whether the update expression for e.g. .update(json_field__key2__key2a=123) should be implemented in the KeyTransform class vs. the JSONField itself. There are implications toward future extensibility for other transforms, so I'd like to get the community's opinions on this.

I've left some comments to add a bit of context that hopefully helps explain the intentions.

laymonage · 2024-09-26T18:16:23Z

django/db/models/functions/comparison.py

+        elif output_type == "JSONField":
+            template = "JSON(%(expressions)s)"
+            return super().as_sql(
+                compiler, connection, template=template, **extra_context
+            )


Reason why we need this:

https://www.sqlite.org/json1.html#jset

If the value of a path/value pair is an SQLite TEXT value, then it is normally inserted as a quoted JSON string, even if the string looks like valid JSON. However, if the value is the result of another json function (such as json() or json_array() or json_object()) or if it is the result of the -> operator, then it is interpreted as JSON and is inserted as JSON retaining all of its substructure. Values that are the result of the ->> operator are always interpreted as TEXT and are inserted as a JSON string even if they look like valid JSON.

It's very similar to the other workarounds in as_mysql for MariaDB and in as_oracle. Instead of putting the workarounds directly in Cast.as_vendor(), do we want to introduce a new database operation specifically for casting JSONField instead? We have something similar for the datetime transforms:

django/django/db/backends/base/operations.py

Lines 131 to 147 in 5ed7208

def datetime_cast_date_sql(self, sql, params, tzname):

"""

Return the SQL to cast a datetime value to date value.

"""

raise NotImplementedError(

"subclasses of BaseDatabaseOperations may require a "

"datetime_cast_date_sql() method."

)

def datetime_cast_time_sql(self, sql, params, tzname):

"""

Return the SQL to cast a datetime value to time value.

"""

raise NotImplementedError(

"subclasses of BaseDatabaseOperations may require a "

"datetime_cast_time_sql() method"

)

laymonage · 2024-09-27T10:46:40Z

django/db/models/functions/json.py

+                # Use Value to serialize the data to string,
+                # then use Cast to ensure the string is treated as JSON.
+                value = Cast(
+                    Value(value, output_field=self.output_field),
+                    output_field=self.output_field,
+                )


Value() with JSONField() would allow us to reuse its get_db_prep_value() which does the serialization of Python objects based on the DB vendor. Then, we wrap it in Cast() because this value will be used as a parameter to the DB's JSON_SET function, and some DBs (e.g. SQLite) will treat the value as a plain SQL string (even if it's valid JSON) if we don't use Cast.

This is unlike saving values directly to the column, in which case the DBs will happily accept a JSON-formatted string and do any conversion to the actual JSON data type (if they have one).

laymonage · 2024-09-27T14:56:49Z

django/db/models/functions/json.py

+            class ToJSONB(Func):
+                function = "TO_JSONB"
+
+            new_source_expressions.append(
+                ToJSONB(value, output_field=self.output_field),
+            )


Casting to JSONB with ::jsonb on Postgres only works for strings (text) that are JSON-formatted. If you have an expression that resolves to a non-string SQL value, e.g. when you refer to an integer column with F(), using Cast() doesn't work. Postgres has a dedicated to_jsonb function for this: https://www.postgresql.org/docs/current/functions-json.html#FUNCTIONS-JSON-CREATION-TABLE

laymonage · 2024-09-27T14:58:46Z

django/db/models/functions/json.py

+        if not hasattr(value, "resolve_expression"):
+            new_source_expressions.append(Value(value, output_field=self.output_field))


We don't need Cast() because Value() with a JSONField is correctly handled as JSONB by Postgres.

laymonage · 2024-09-27T15:23:26Z

django/db/models/functions/json.py

+                if not hasattr(value, "resolve_expression"):
+                    # Interpolate the JSON path directly to the query string, because
+                    # Oracle does not support passing the JSON path using parameter
+                    # binding.
+                    return f"{args[0]}, SET '{key_paths_join}' = {args[-1]} FORMAT JSON"
+                return f"{args[0]}, SET '{key_paths_join}' = {args[-1]}"


Interpolating the JSON path directly is nothing new:

django/django/db/models/fields/json.py

Lines 235 to 237 in 5ed7208

# Add paths directly into SQL because path expressions cannot be passed

# as bind variables on Oracle.

return sql % tuple(params), []

Though I think we should move the comment to be outside the if, because this applies to both cases.

Suggested change

if not hasattr(value, "resolve_expression"):

# Interpolate the JSON path directly to the query string, because

# Oracle does not support passing the JSON path using parameter

# binding.

return f"{args[0]}, SET '{key_paths_join}' = {args[-1]} FORMAT JSON"

return f"{args[0]}, SET '{key_paths_join}' = {args[-1]}"

# Interpolate the JSON path directly to the query string, because

# Oracle does not support passing the JSON path using parameter

# binding.

if not hasattr(value, "resolve_expression"):

return f"{args[0]}, SET '{key_paths_join}' = {args[-1]} FORMAT JSON"

return f"{args[0]}, SET '{key_paths_join}' = {args[-1]}"

Using FORMAT JSON is similar to using Cast() in other DBs but this is specific to Oracle's JSON_TRANSFORM.

https://docs.oracle.com/en/database/oracle/oracle-database/21/adjsn/oracle-sql-function-json_transform.html

If the result expression evaluates to a SQL value that is not JSON type, you can convert it to JSON data by following the expression immediately with keywords FORMAT JSON. This is particularly useful to convert the SQL string 'true' or 'false' to the corresponding JSON-language value true or false. Example 11-7 illustrates this.

laymonage · 2024-09-27T16:30:53Z

tests/update/tests.py

        self.assertEqual(related.name, "new")
+
+
+class TransformUpdateTests(TestCase):


At the moment, these tests are not particularly useful for real-life cases. However, this ensures if you try to use update() with transforms that do not support it yet, a helpful error message is thrown (instead of some unexpected exception/undefined behaviour).

laymonage · 2024-09-27T16:41:41Z

django/db/models/fields/json.py

+    def get_update_expression(self, value, lhs=None):
+        from ..functions import JSONRemove, JSONSet
+
+        field, key_transforms = self.unwrap_transforms()


There's a bit of round trip around the transforms, so let me explain the flow.

We do MyModel.objects.update(json_field__key1="foo", json_field__key2__key2a=123)

QuerySet.update() will construct an UpdateQuery which calls add_update_values() to prepare the expressions for each field that we are updating.

In add_update_values(), we'll split the kwargs using LOOKUP_SEP.

Then, we ensure each name in the resulting split are valid transforms by doing a get_transform() on the field with each string to get the transform class. If it is found, we'll construct the transform. In JSONField's case, it will always give you KeyTransform (because keys can be of any string).

If the transform is chained, we keep on building the transforms by composing them, e.g. KeyTransform(KeyTransform(...)). For other fields, this may not be of the same class, e.g. TransformA(TransformB(...)). There's no concrete example for this right now, though.

After constructing the final Transform instance (the last one from the result of .split(LOOKUP_SEP)), we call its get_update_expression() method.

In KeyTransform, we use the unwrap_transforms so that we can "unwrap" the transform composition e.g. KeyTransform("key2a", KeyTransform("key2", "json_field")) back into field, ["key2", "key2a"].

Then, we join the keys with LOOKUP_SEP again, so we can pass it to JSONSet()/JSONRemove() as JSONSet(field, key2__key2a=value).

Then JSONSet()/JSONRemove() is going to – yet again –, split the kwargs with LOOKUP_SEP, because it needs to build the final JSON path string e.g. $.key2.key2a.

We can probably optimize the last bits by introducing a classmethod on JSONSet/JSONRemove that accepts a pair of a KeyTransform instance and the value...

Alternatively, we could also make it so that the field itself, rather than the transform, defines the expression. This lets us skip the round trip of splitting/joining on LOOKUP_SEP entirely.

For example, if this get_update_expression is implemented in JSONField (instead of KeyTransform), we could just pass the kwargs from .update() directly to the JSONSet() and JSONRemove() functions. This skips KeyTransform entirely.

This approach will result in a much more straightforward code flow. However, it would mean that any use of LOOKUP_SEP in update() is handled entirely by the field itself, rather than going through the transforms mechanism like in .filter().

I think, to make this extensible and more consistent with .filter(), we should do the transforms approach, even if it means doing a bit of round trips in the case of KeyTransform.

laymonage · 2024-09-27T16:44:43Z

django/db/models/sql/subqueries.py

-        values_seq = []
+        field_dict = dict()
        for name, val in values.items():
+            name, *transforms = name.split(LOOKUP_SEP)


This method is used by QuerySet.update(), as it constructs an UpdateQuery to build the SQL UPDATE steatement. Previously, Django never split the kwargs on LOOKUP_SEP (unlike in .filter()) because LOOKUP_SEP is normally used for:

referring to a field in a related model

doing lookups and transforms
and both of which are currently unsupported in update().

AFAIK, .update() can only update the table of the model itself, and not the table of a related model (because a single SQL UPDATE statement can only update one table) – so this is not something we need to consider.

Lookups only make sense in the context of making conditionals, e.g. gte, startswith. It doesn't make sense to use them in the context of update(), e.g. what does it mean to do MyModel.objects.update(some_integerfield__gte=42)?

For transforms, they make sense in both cases. You can already do MyModel.objects.filter(some_datefield__year=2023). Adding support for something like .update(some_datefield__year=2024) doesn't seem too far-fetched.

So, in here, we add .split(LOOKUP_SEP) and assume that the rest that follow the first element are transforms.

laymonage · 2024-09-27T17:23:38Z

tests/db_functions/json/test_json_set.py

+
+
+@skipUnlessDBFeature("supports_partial_json_update")
+class JSONSetTests(TestCase):


I think many of the test methods here can be combined into a single method with subTest.

laymonage · 2024-09-27T17:34:37Z

tests/db_functions/json/test_json_set.py

+
+        # It should append the value to the array if the database supports it.
+        if not connection.features.json_set_array_append_requires_length_as_index:
+            if connection.vendor == "oracle":


What happens here is that we're testing the behaviour of JSON_SET when updating an array with an index that's out-of-range. For example, with JSON {"items": ["a","b","c"]}.

If we do JSONSet(field, items__3='d'), this results in {"items": ["a","b","c","d"]} on all databases.

If we do JSONSet(field, items__4='d'), however, this gives different results:

On SQLite, {"items": ["a","b","c"]}. The append does not happen.

On Oracle, {"items": ["a","b","c",null,"d"]}. It fills out the "gap" with null, before appending "d" to the correct index.

On other databases, {"items": ["a","b","c","d"]}. It just appends to the last index of the array.

We weren't sure if this is worth adding another feature flag, so we kind of combined Oracle and SQLite's behaviour into a single json_set_array_append_requires_length_as_index flag, with the base assumption being that the append does not happen, and Oracle being the outlier.

Perhaps, we should only do the full array assertion inside a vendor check for both oracle and sqlite. Then, add a general assertion that field["items"][3] does not equal "d" – this is true for both Oracle and SQLite, and we still leave room for external DB backends which may have the flag also set to true but we don't know the exact behaviour.

laymonage · 2024-09-28T16:51:47Z

@bmispelon

sarahboyce

Thank you @adzshaf 🚀 this looks really strong

I had a good catch up with @laymonage so I am a little up to speed

I have added a couple of notes 👍

sarahboyce · 2024-10-08T14:24:58Z

docs/topics/db/queries.txt

+
+Updating ``JSONField``
+======================
+


Suggested change

.. versionadded:: 5.2

sarahboyce · 2024-10-08T14:27:06Z

docs/ref/models/database-functions.txt

+
+``JSONRemove``
+--------------
+


Suggested change

.. versionadded:: 5.2

Also think it's worth writing a release note for JSONRemove in the minor features as part of b90bf8f

sarahboyce · 2024-10-08T14:27:18Z

docs/ref/models/database-functions.txt

+
+``JSONSet``
+-----------
+


Suggested change

.. versionadded:: 5.2

I also think it might be worth adding a release not for JSONSet in the "Minor features" section as part of this commit (3600f05)

sarahboyce · 2024-10-08T14:50:55Z

tests/update/tests.py

+                "notifications": {"email": False, "sms": True},
+            },
+        )
+


So I was curious what happens when we do update_or_create using this syntax and the answer is, it doesn't fail but it doesn't update (the following test fails expecting it would update).

Suggested change

def test_update_or_create_json_key_transform(self):

user_preferences = UserPreferences.objects.create(

settings={

"theme": {"color": "black", "font": "Arial"},

"notifications": {"email": False, "sms": True},

}

)

updated_user_preferences, created = UserPreferences.objects.update_or_create(

defaults={"settings__theme__color": "white"},

id=user_preferences.id

)

self.assertEqual(created, False)

self.assertEqual(

updated_user_preferences.settings,

{

"theme": {"color": "white", "font": "Arial"},

"notifications": {"email": False, "sms": True},

},

)

Can you investigate whether adding support for update_or_create feasible?

Thanks Sarah, that's a good point! We haven't investigated into adding transforms support for update_or_create, but thanks to your review we looked into how JSONSet and JSONRemove behaves with update_or_create().

We're going to open a separate PR for adding those two functions. In the meantime here are our findings for using update_or_create with the functions.

With the following example:

updated_user_preferences, created = UserPreferences.objects.update_or_create( defaults={"settings": JSONSet("settings", theme__color="white")}, id=user_preferences.id, )

In the case where it does an update (created is False), it worked well – the test passed.

In the case where it does an insert (created is True), it fails because we'd be referring to the column for the row which hasn't been inserted yet. It makes sense that this would fail, but I think it would be nice if JSONSet() and JSONRemove() can fall back to an empty JSON object as the starting point in the insert case.

The main blocker we found in this case is that the for_save parameter in BaseExpression.resolve_expression() doesn't let you differentiate whether it's an INSERT or an UPDATE query – it's set to True in both cases.

We hacked the places where we pass for_save=True and changed it to pass the strings "create" and "update" depending on the query, and we were able to make JSONSet() and JSONRemove() resolve the expression accordingly. See adzshaf@207598f for more details.

I'm not sure the best course of action on that, though. That being said, I just noticed this forum thread about atomic upserts that would also benefit from the ability to differentiate between INSERT vs UPDATE in resolve_expression(). I'll ask there and see if others have an opinion.

laymonage

Just a quick update: @adzshaf and I still work on this regularly every week (we just haven't pushed the updates to the PR branch). We're going to split this PR into two:

Adding JSONSet() and JSONRemove()
Adding support for using key transforms in QuerySet.update() and QuerySet.update_or_create() for JSONFields (which uses JSONSet and JSONRemove under the hood)

Along with a few more tweaks based on @sarahboyce's feedback at the DjangoCon US sprints. We found a few more edge cases so it's taking us quite a bit of time, but hopefully we'll open a new PR soon.

laymonage · 2024-10-25T11:31:39Z

tests/update/tests.py

+                "notifications": {"email": False, "sms": True},
+            },
+        )
+


Thanks Sarah, that's a good point! We haven't investigated into adding transforms support for update_or_create, but thanks to your review we looked into how JSONSet and JSONRemove behaves with update_or_create().

We're going to open a separate PR for adding those two functions. In the meantime here are our findings for using update_or_create with the functions.

With the following example:

updated_user_preferences, created = UserPreferences.objects.update_or_create( defaults={"settings": JSONSet("settings", theme__color="white")}, id=user_preferences.id, )

In the case where it does an update (created is False), it worked well – the test passed.

In the case where it does an insert (created is True), it fails because we'd be referring to the column for the row which hasn't been inserted yet. It makes sense that this would fail, but I think it would be nice if JSONSet() and JSONRemove() can fall back to an empty JSON object as the starting point in the insert case.

The main blocker we found in this case is that the for_save parameter in BaseExpression.resolve_expression() doesn't let you differentiate whether it's an INSERT or an UPDATE query – it's set to True in both cases.

We hacked the places where we pass for_save=True and changed it to pass the strings "create" and "update" depending on the query, and we were able to make JSONSet() and JSONRemove() resolve the expression accordingly. See adzshaf@207598f for more details.

I'm not sure the best course of action on that, though. That being said, I just noticed this forum thread about atomic upserts that would also benefit from the ability to differentiate between INSERT vs UPDATE in resolve_expression(). I'll ask there and see if others have an opinion.

charettes

Following the ping from this forum thread from @laymonage

I think the idea of making for_save a (enum.Enum, str) to differentiate between different scenario could could as most usage out there are doing if for_save and most likely don't need to differentite between both.

My immediate concerns with these changes are that they are possibly trying to do too much at once?

Is there a reason why we can't first land support for JSONSet / JSONRemove in a first set of changes targeted for 5.2, let them simmer for a release, and then consider adding support for the transform combining feature later on?

I like the ideas put forward by this patch and the separation of concern between field and transform logic but these are more fundamental changes to the ORM works and bring larger questions. For example, should we support save(update_fields) with transforms as well? Is it time to introduce a specialized type of transform that denotes nested data structure access instead of trying to push the current abstraction in this space?

IMO these questions can likely be more easily answered while users are allowed to play with JSONSet and JSONRemove and these primitives are iterated on to iron out bugs first.

charettes · 2024-10-26T13:23:19Z

django/db/models/fields/json.py

+        if lhs is None:
+            lhs = field
+
+        if value is NOT_PROVIDED:


I like the idea of using a sentinel to differentiate between null and key deletions (it's something that I believe is missing for JSON null vs SQL NULL) bet I'm not sure we should piggy back on that one since it was meant for a completely different purpose.

I agree @charettes! This was a temporary solution as suggested by Markus the other day, and I linked to your exact comment: https://fosstodon.org/@laymonage/112949402890177435 (Like I said, big fan 😂)

Happy to introduce a new sentinel for this, although if we want to also support that sentinel in JSONField at the same time, that sounds like a separate work in itself.

I think the idea of making for_save a (enum.Enum, str) to differentiate between different scenario could could as most usage out there are doing if for_save and most likely don't need to differentite between both.

That's what I thought too – though I suppose it could still break if people check for the boolean explicitly e.g. if for_save is True. Still doable, just need to be more careful when we do the transition.

My immediate concerns with these changes are that they are possibly trying to do too much at once?

Is there a reason why we can't first land support for JSONSet / JSONRemove in a first set of changes targeted for 5.2, let them simmer for a release, and then consider adding support for the transform combining feature later on?

No particular reason, really. When we implemented the first pass for the JSONSet and JSONRemove functions, we were curious how much work it would take to implement the transforms support in update() directly. Turns out it wasn't much, but indeed it's a bit more "controversial".

Sarah and I discussed this at the DjangoCon US sprints and we decided to split this PR into two. One for the DB functions, and one for the nicer syntactic sugar via update() directly – just like you said. I noted this in my comment #18489 (review) just yesterday.

@adzshaf and I are still working on polishing the DB functions to be ready for a new, smaller PR. We found a few more edge cases so it's taking us a bit longer than we thought. We'll post an update on the ticket when we put the PR up 😄

I like the ideas put forward by this patch and the separation of concern between field and transform logic but these are more fundamental changes to the ORM works and bring larger questions. For example, should we support save(update_fields) with transforms as well? Is it time to introduce a specialized type of transform that denotes nested data structure access instead of trying to push the current abstraction in this space?

IMO these questions can likely be more easily answered while users are allowed to play with JSONSet and JSONRemove and these primitives are iterated on to iron out bugs first.

Thank you, and yes there are definitely more areas of the ORM that we haven't considered like bulk_{create,update}, update_or_create, etc. and save(update_fields) is one of them. I'd be happy to focus on getting JSONSet and JSONRemove in first, then discuss the more uncertain bits after.

Thanks for the update, it's great to see you push all of these JSONField improvement forward. It's has been a game changer to be able to use JSONField for non-Postgres database and a lot of that is thanks to you 🙇

I think that the two-commit approach has a lot of merit considering how hard it is to get these things right the first time. Let me know when things have shappen if you'd like more feedback. As I said already I think the proposed approach is great in the sense that it demonstrates that it can be done without too many invasive changes.

adzshaf · 2024-11-03T14:18:36Z

Just like what @laymonage mentioned, I have created separate PR for adding JSONSet and JSONRemove:

Refs #32519 -- Added JSONSet and JSONRemove functions. #18758

Once that PR is merged, we will revisit the support for using key and path transforms in update() in a new PR.

Thank you @sarahboyce @charettes for the reviews ❤️

laymonage suggested changes Aug 20, 2024

View reviewed changes

adzshaf force-pushed the ticket_32519 branch from f2358a4 to d13ad71 Compare August 24, 2024 16:19

adzshaf force-pushed the ticket_32519 branch 3 times, most recently from 6eb6c8f to 7200b96 Compare September 14, 2024 15:39

adzshaf added 8 commits September 26, 2024 20:46

Added JSONField handling for Cast() on SQLite.

d4ba266

This allows us to mark serialized JSON data as JSON instead of a regular string when passing it to the database.

Added JSONSet for making partial updates to JSONField.

3600f05

Added JSONRemove for removing keys in JSONField.

b90bf8f

Extracted KeyTransform.unwrap_transforms() from KeyTransform.preproce…

5e71358

…ss_lhs().

Added Transform.get_update_expression() to be used in QuerySet.update().

67dc0f9

Added support for using JSONField key transforms in QuerySet.update().

3973d4e

Added topic guide for updating JSONField.

bba67d6

Added release notes for partial JSONField updates support.

f6f8148

adzshaf force-pushed the ticket_32519 branch from 7200b96 to f6f8148 Compare September 26, 2024 13:48

laymonage reviewed Sep 27, 2024

View reviewed changes

sarahboyce reviewed Oct 8, 2024

View reviewed changes

laymonage reviewed Oct 25, 2024

View reviewed changes

charettes reviewed Oct 26, 2024

View reviewed changes

adzshaf closed this Nov 3, 2024

adzshaf mentioned this pull request Nov 3, 2024

Refs #32519 -- Added JSONSet and JSONRemove functions. #18758

Open

6 tasks

glennmatthews mentioned this pull request Nov 13, 2024

Adopt latest recommendations from upstream for JSONSet and JSONRemove funcs nautobot/nautobot#6515

Open

	def datetime_cast_date_sql(self, sql, params, tzname):
	"""
	Return the SQL to cast a datetime value to date value.
	"""
	raise NotImplementedError(
	"subclasses of BaseDatabaseOperations may require a "
	"datetime_cast_date_sql() method."
	)

	def datetime_cast_time_sql(self, sql, params, tzname):
	"""
	Return the SQL to cast a datetime value to time value.
	"""
	raise NotImplementedError(
	"subclasses of BaseDatabaseOperations may require a "
	"datetime_cast_time_sql() method"
	)

		if not hasattr(value, "resolve_expression"):
		new_source_expressions.append(Value(value, output_field=self.output_field))

	# Add paths directly into SQL because path expressions cannot be passed
	# as bind variables on Oracle.
	return sql % tuple(params), []

		self.assertEqual(related.name, "new")


		class TransformUpdateTests(TestCase):



		@skipUnlessDBFeature("supports_partial_json_update")
		class JSONSetTests(TestCase):

+    def test_update_or_create_json_key_transform(self):
+        user_preferences = UserPreferences.objects.create(
+            settings={
+                "theme": {"color": "black", "font": "Arial"},
+                "notifications": {"email": False, "sms": True},
+            }
+        )
+        updated_user_preferences, created = UserPreferences.objects.update_or_create(
+            defaults={"settings__theme__color": "white"},
+            id=user_preferences.id
+        )
+        self.assertEqual(created, False)
+        self.assertEqual(
+            updated_user_preferences.settings,
+            {
+                "theme": {"color": "white", "font": "Arial"},
+                "notifications": {"email": False, "sms": True},
+            },
+        )


		Updating ``JSONField``
		======================


		``JSONRemove``
		--------------


		``JSONSet``
		-----------

Uh oh!

Fixed #32519 -- Added support for using key and path transforms in update() for JSONFields. #18489

Fixed #32519 -- Added support for using key and path transforms in update() for JSONFields. #18489

Uh oh!

Conversation

adzshaf commented Aug 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Trac ticket number

Branch description

Checklist

Uh oh!

laymonage left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laymonage left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laymonage commented Sep 28, 2024

Uh oh!

sarahboyce left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laymonage left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charettes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

adzshaf commented Aug 17, 2024 •

edited

Loading

laymonage left a comment •

edited

Loading

laymonage left a comment •

edited

Loading

charettes left a comment •

edited

Loading

laymonage Oct 26, 2024 •

edited

Loading