Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#36453 closed Bug (fixed)

5.2.3 introduces a regression when using `Value(None, output_field=JSONField()` in a `When` clause.

Reported by: Thomas Owned by: Clifford Gama
Component: Database layer (models, ORM) Version: 5.2
Severity: Release blocker Keywords:
Cc: Thomas Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Thomas)

We've tried to upgrade to 5.2.3 and several of our tests started failing. We've pinpointed the problem to one query which uses Value(None, output_field=JSONField() in a When clause. Here's a minimal reproducible example (with a WHERE clause too, to highlight the difference in behavior):

from django.db import connection
from django.db.models import (
    BigIntegerField,
    Case,
    F,
    JSONField,
    Model,
    Value,
    When,
)
from django.db.models.functions import Cast


class Foo(Model):
    bar = BigIntegerField(blank=True, null=True)
    json_field = JSONField(blank=True, null=True)


if __name__ == '__main__':
    Foo.objects.filter(
        json_field__key_1=Value(None, output_field=JSONField())
    ).update(
        bar=Case(
            When(
                json_field__key_2=Value(None, output_field=JSONField()),
                then=None
            ),
            default=Cast(F('json_field__key_2'), BigIntegerField()),
            output_field=BigIntegerField(),
        ),
    )

    print(connection.queries[-1]['sql'])

We ran this code with 5.2.2 (with this version our test suite passes) and 5.2.3 (with this version our test suite fails) and got this:

-- Generated with Django 5.2.2
UPDATE "polls_foo"
SET "bar" = CASE WHEN (
        ("polls_foo"."json_field" -> 'key_2') = 'null'::jsonb  -- JSON "null"
    )
    THEN NULL
    ELSE (("polls_foo"."json_field" -> 'key_2'))::bigint
    END
WHERE ("polls_foo"."json_field" -> 'key_1') = 'null'::jsonb;

-- Generated with Django 5.2.3
UPDATE "polls_foo"
SET "bar" = CASE WHEN (
        ("polls_foo"."json_field" -> 'key_2') = NULL  -- SQL NULL
    )
    THEN NULL
    ELSE (("polls_foo"."json_field" -> 'key_2'))::bigint
    END
WHERE ("polls_foo"."json_field" -> 'key_1') = 'null'::jsonb;

Notice how the WHEN clause differs, but the WHERE clause stays the same, despite both using the same Value(None, output_field=JSONField()) syntax.

Edit: If that makes any difference, we're using PostgreSQL as a backend.

Change History (10)

comment:1 by Thomas, 3 months ago

Description: modified (diff)

comment:2 by David Sanders, 3 months ago

Resolution: duplicate
Status: newclosed

Duplicate of #36445 👍

comment:3 by Thomas, 3 months ago

I guess this was closed as duplicate because the cause might be the same, but then I don't understand how the other ticket claims it isn't a regression. The code I posted has been working since 4.x at least. It only breaks with 5.2.3 (released June 10th), which I guess was released with a known bug/regression (#36445, June 6th).

Last edited 3 months ago by Thomas (previous) (diff)

comment:4 by Clifford Gama, 3 months ago

Resolution: duplicate
Severity: NormalRelease blocker
Status: closednew
Triage Stage: UnreviewedAccepted

This is indeed a regression in c1fa3fdd040718356e5a3b9a0fe699d73f47a940.

I think the problem is that we are now passing on for_save=True from the update to Case's source expressions, which ends up resolving When.condition with for_save=True.

The issue is related to #36445 in that they both expose problems related to the use of Value(None, JSONField()) to mean JSON null and None to mean NULL, and how that is implemented depending on whether JSONFIeld.get_db_prep_save is called. I think these set of tickets should be an argument in favour of introducing a JSONNull value as outlined in ticket:35381#comment:5.

Last edited 3 months ago by Clifford Gama (previous) (diff)

comment:5 by Clifford Gama, 3 months ago

Has patch: set

comment:6 by Thomas, 3 months ago

Thank you for the super fast patch, @Clifford Gama.

comment:7 by Sarah Boyce, 3 months ago

Owner: set to Clifford Gama
Status: newassigned

comment:8 by Sarah Boyce, 3 months ago

Triage Stage: AcceptedReady for checkin

comment:9 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

Resolution: fixed
Status: assignedclosed

In 104cbfd4:

Fixed #36453 -- Made When.condition resolve with for_save=False.

Value(None, JSONField()) when used in When.condition incorrectly resolved with
for_save=True, resulting in the value being serialized as SQL NULL instead of
JSON null.

Regression in c1fa3fdd040718356e5a3b9a0fe699d73f47a940.

Thanks to Thomas McKay for the report, and to David Sanders and Simon Charettes
for the review.

Co-authored-by: Sarah Boyce <42296566+sarahboyce@…>

comment:10 by Sarah Boyce <42296566+sarahboyce@…>, 3 months ago

In 1d89691:

[5.2.x] Fixed #36453 -- Made When.condition resolve with for_save=False.

Value(None, JSONField()) when used in When.condition incorrectly resolved with
for_save=True, resulting in the value being serialized as SQL NULL instead of
JSON null.

Regression in c1fa3fdd040718356e5a3b9a0fe699d73f47a940.

Thanks to Thomas McKay for the report, and to David Sanders and Simon Charettes
for the review.

Co-authored-by: Sarah Boyce <42296566+sarahboyce@…>

Backport of 104cbfd44b9eff010daf0ef0e1ce434385855b13 from main.

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