bulk_update() incorrectly updates JSONField None to JSON 'null' instead of SQL NULL

[Clifford Gama]

Failing test:

tests/queries/test_bulk_update.py

@@ -300 +300 @@
             JSONFieldNullable.objects.filter(json_field__has_key="c"), objs
         )
 
+    @skipUnlessDBFeature("supports_json_field")
+    def test_json_field_none_value(self):
+        """Top level None value must be equivalent to SQL NULL in db."""
+        obj = JSONFieldNullable.objects.create(json_field={"k": "v"})
+        obj.json_field = None
+        JSONFieldNullable.objects.bulk_update([obj], fields=["json_field"])
+        obj.refresh_from_db()
+        qs = JSONFieldNullable.objects.filter(json_field__isnull=True)
+        self.assertSequenceEqual(qs, [obj])
+
     def test_nullable_fk_after_related_save(self):
         parent = RelatedObject.objects.create()
         child = SingleObject()

This is inconsistent with create(), update() and save() (see ​test and ​docs), and can cause unexpected behaviour when a JSONField has null=False, since JSON null does not violate null=False.

[Natalia Bidart]

Hello Clifford, thank you for your investigation on #36418 and for creating this ticket. If you ask me, I think the reporter in #36418 was reporting this exact same thing, but I also agree the report was a bit confusing (since it was not stated what was the expected outcome). I'll be accepting this ticket based on that, and also because for models with a null=False JSONField, this code does raise IntegrityError:

obj = JSONModel()                                                      
obj.value = None                                                       
obj.save()                                                             

but bulk_update does not.

[Clifford Gama]

Thanks for the triage, Natalia.

[Jacob Walls]

#36440 was a dupe. Adam bisected it to 5.2 commit e306687a3a5507d59365ba9bf545010e5fd4b2a8, so elevating to blocker for now.

[Natalia Bidart]

For what is worth, the test_json_field_json_null_value as presented in the proposed branch fails in 5.1.x, 4.2.x, 4.1.x, 4.0.x and 3.2.x, so unclear this is truly a release blocker for 5.2.x.

Adam, could you provide more details about your bisect outcome?

[Clifford Gama]

Hello Natalia. I did a git bisect for test_json_field_sql_null_value and the issue is indeed a regression in 00c690efbc0b10f67924687f24a7b30397bf47d9. The test by test_json_field_json_null_value is something I had assumed to have always worked and isn't actually a regression test for the issue in this ticket.

It seems bulk_update() was creating SQL NULL even for Value(None, output_field=JSONField()) maybe since JSONField was introduced.

[Adam Johnson]

Adam, could you provide more details about your bisect outcome?

I bisected for my client project failure, so unfortunately, I can't really share that. I found that the client test failed for e306687a3a5507d59365ba9bf545010e5fd4b2a8.

I then minimized the issue into the test provided in ​https://github.com/django/django/pull/19527 , which reproduces the exception I encountered. I didn't bisect with that test before.

I just tried bisecting with it and found 1df0f998ae8d1626adaa9b807a43e4cbdfc590e2, the 5.2 backport of 00c690efbc0b10f67924687f24a7b30397bf47d9, is indeed the first commit that makes my test fail.

So it seems we're working on the same issue, but it may be that there may be some other failure lurking in the client project test. I'll try to check, but no promises!

[Sarah Boyce <42296566+sarahboyce@…>]

In c1fa3fdd:

Fixed #36419 -- Ensured for_save was propagated when resolving expressions.

The for_save flag wasn't properly propagated when resolving expressions, which
prevented get_db_prep_save() from being called in some cases. This affected
fields like JSONField where None would be saved as JSON null instead of SQL NULL.

Regression in 00c690efbc0b10f67924687f24a7b30397bf47d9.

Thanks to David Sanders and Simon Charette for reviews.

Co-authored-by: Adam Johnson <me@…>

[Sarah Boyce <42296566+sarahboyce@…>]

In 6fc620b4:

[5.2.x] Fixed #36419 -- Ensured for_save was propagated when resolving expressions.

The for_save flag wasn't properly propagated when resolving expressions, which
prevented get_db_prep_save() from being called in some cases. This affected
fields like JSONField where None would be saved as JSON null instead of SQL NULL.

Regression in 00c690efbc0b10f67924687f24a7b30397bf47d9.

Thanks to David Sanders and Simon Charette for reviews.

Co-authored-by: Adam Johnson <me@…>

Backport of c1fa3fdd040718356e5a3b9a0fe699d73f47a940 from main.

[GitHub <noreply@…>]

In f5441e42:

Refs #36419 -- Fixed BulkUpdateTests.test_json_field_sql_null() crash on Oracle.

Follow up to c1fa3fdd040718356e5a3b9a0fe699d73f47a940.

[Mariusz Felisiak <felisiak.mariusz@…>]

In aec11db:

[5.2.x] Refs #36419 -- Fixed BulkUpdateTests.test_json_field_sql_null() crash on Oracle.

Follow up to c1fa3fdd040718356e5a3b9a0fe699d73f47a940.

Backport of f5441e42da691ee2e7aeeb9be70f98e2bce6d17d from main.