Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in bulk_update()

[Clifford Gama]

When using bulk_update() with a Value(None, output_field=JSONField()) expression, the value is incorrectly saved as a SQL NULL rather than a JSON null. This behavior was temporarily fixed in 00c690efbc0b10f67924687f24a7b30397bf47d9 due to incomplete for_save propagation during expression resolution (bug in #36419), but reappears after the proper restoration of for_save handling in #36419.

tests/queries/test_bulk_update.py

@@ -3 +3 @@
 
 from django.core.exceptions import FieldDoesNotExist
 from django.db import connection
-from django.db.models import F
+from django.db.models import F, Value, JSONField
 from django.db.models.functions import Coalesce, Lower
 from django.db.utils import IntegrityError
 from django.test import TestCase, override_settings, skipUnlessDBFeature
@@ -315 +315 @@
                 sql_null_qs = JSONFieldNullable.objects.filter(json_field__isnull=True)
                 self.assertSequenceEqual(sql_null_qs, [obj])
 
+    @skipUnlessDBFeature("supports_json_field")
+    def test_json_field_json_null_value(self):
+        obj = JSONFieldNullable.objects.create(json_field={})
+        obj.json_field = Value(None, output_field=JSONField())
+        JSONFieldNullable.objects.bulk_update([obj], fields=["json_field"])
+        obj.refresh_from_db()
+        json_null_qs = JSONFieldNullable.objects.filter(json_field=None)
+        self.assertSequenceEqual(json_null_qs, [obj])
+
     def test_nullable_fk_after_related_save(self):
         parent = RelatedObject.objects.create()
         child = SingleObject()

[Sarah Boyce]

Thank you for the ticket

[Simon Charette]

Now that we know this isn't a regression should we consider biting the bullet and stop trying to make Value(None, JSONField()) work by introducing a proper JSONNull expression as described in ticket:35381#comment:3?

[GoyalRocks007]

Hello this ticket was unassigned so I assigned this to myself as mentioned in the documentation. I have raised a [PR ​https://github.com/django/django/pull/19546] for this bug. Please review. I have tried to follow all the instructions as mentioned in the documentation. Please let me know if I missed anything.
Thanks!

[Thomas]

@Simon Charette: It may not be a regression for the bulk_update case outlined here (of which I don't fully understand the chronology), but isn't it a regression in the case I outlined in #36453 (which has been marked as a duplicate of this issue)? Code that has been working a certain way for 1-2 years is broken in 5.2.3.

[Simon Charette]

Now that we know this isn't a regression

I wasn't involved in the regression status determination, my comment was referring to ​this discussion and assuming it was the case. It is possible that the issue you reported in #36453 is a valid regression in Django 5.2.3 for a slightly different scenario as When / Case have a variation of expression resolving than Coalesce.

If you can write a regression test that passes for < 5.2 and fail for 5.2.3 then it is effectively a regression and closing #36453, and potentially categorizing this ticket as an issue predating 5.2, was likely a triage mistake.

[Thomas]

I've made a regression test for the use case detailed in #36453.
​https://github.com/thomas-mckay/django/commit/5ca2b19568b39c41248479ad4b19973fe622c438

After bisection, I've noticed that the test passes from 4.2 to 5.2.2. It breaks in 5.2.3 and before 4.2.
To recap:

• ​https://github.com/django/django/commit/5c23d9f0c32f166c81ecb6f3f01d5077a6084318 was introduced in 4.2. Before this commit the test fails, after this commit the test passes, until 5.2.3
• ​https://github.com/django/django/commit/6fc620b4a8e91839b93af2b52d80bdbd5f8a1fcc was introduced in 5.2.3. After this commit the test fails again.

I've checked Value.for_save, and it is indeed False in 5.2.2 and True in 5.2.3 for the Value in the When condition. So, I understand why #36453 may have been closed as a duplicate of this one: it sounds like the same change may be responsible. My only concern is that the bug in this ticket only covers one of the side-effects of that change (that may or may not be a regression), but not the one I'm experiencing (which I really hope is considered a regression), and that addressing this side-effect will not fix the behavior that's been broken in my case. Let me know if I'm taking this ticket off-topic and I'll re-open my ticket and continue there.

[Thomas]

I've tested the patch proposed by @GoyalRocks007 in comment:5. With this patch the test passes again.

[Clifford Gama]

Replying to Simon Charette:

Now that we know this isn't a regression should we consider biting the bullet and stop trying to make Value(None, JSONField()) work by introducing a proper JSONNull expression as described in ticket:35381#comment:3?

In light of recent reports around JSON null/SQL NULL, (the latest being #36453) I'd say yes. Would this mean marking this ticket as invalid in favour of a #35381 repurposed for the JSONNull new feature? Or perhaps a new ticket?

[Sarah Boyce]

I see that there is a growing consensus to pursue JSONNull (ref ticket:36445#comment:4, ticket:35381#comment:10, ticket:36445#comment:11).
We should pursue this before bolting on more logic to trying to resolve the json null problem (note that I confirmed that this test in the PR has been failing since at least Django 4.1 and potentially has never worked)

[Natalia Bidart]

#35381 was repurposed so this ticket is being closed as wontfix following previous conversations.