[Django] #36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in bulk_update()

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
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.

{{{#!diff
djGHOST@anonymous:~/code/django$ git diff
diff --git a/tests/queries/test_bulk_update.py
b/tests/queries/test_bulk_update.py
index 480fac6784..6c553421cb 100644
--- a/tests/queries/test_bulk_update.py
+++ b/tests/queries/test_bulk_update.py
@@ -3,7 +3,7 @@ from math import ceil

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,6 +315,15 @@ class BulkUpdateTests(TestCase):
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()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36445>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------

Reporter: Clifford Gama | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Thank you for the ticket
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:1>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Clifford Gama:

Old description:

New description:

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.

{{{#!diff

--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:2>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------

Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GoyalRocks007):

* owner: (none) => Uddyan Goyal
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:3>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:4>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GoyalRocks007):

* has_patch: 0 => 1

Comment:

Hello this ticket was unassigned so I assigned this to myself as mentioned
in the documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:5>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.1.3.
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:6>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* cc: Simon Charette (added)

Comment:

> Now that we know this isn't a regression

I wasn't involved in the regression status determination, my comment was
referring to
[https://github.com/django/django/pull/19503#discussion_r2130245705 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 fails pass 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:7>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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/d87a7b9f4b4c75fc03ce6bbf55c880a79d524306
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: the
same change is responsible. My only concern is that the bug in this ticket
only covers one of the symptoms 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 symptom 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:8>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Thomas):

I've tested the patch proposed by @GoyalRocks007 in comment:5. With this
patch the test passes again.
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:9>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Thomas):

* cc: Thomas (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:10>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Clifford Gama):

Replying to [comment:4 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:11>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

Comment:

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)
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:12>

[Django]

#36445: Value(None, output_field=JSONField()) incorrectly saves as SQL NULL in
bulk_update()
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Uddyan
| Goyal

Type: Bug | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* resolution: => wontfix
* status: assigned => closed

Comment:

#35381 was repurposed so this ticket is being closed as `wontfix`
following previous conversations.
--
Ticket URL: <https://code.djangoproject.com/ticket/36445#comment:13>