Opened 13 days ago

Last modified 13 days ago

#35381 new Bug

Regression on json null value constraints in django 4.2

Reported by: Olivier Tabone Owned by: nobody
Component: Database layer (models, ORM) Version: 5.0
Severity: Normal Keywords:
Cc: David Sanders, Simon Charette, Mariusz Felisiak Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Regression is still present in 5.0 and main branch

given a model defined as follow

from django.db import models


class JSONFieldModel(models.Model):
    data = models.JSONField(null=True, blank=True, default=None)

    class Meta:
        required_db_features = {"supports_json_field"}
        constraints = [
            models.CheckConstraint(
                check=~models.Q(data=models.Value(None, models.JSONField())),
                name="json_data_cant_be_json_null",
            ),
        ]

the following test works in django 4.1.13, fails in later version

from django.test import TestCase, skipUnlessDBFeature

from .models import JSONFieldModel


class CheckConstraintTests(TestCase):
    @skipUnlessDBFeature("supports_json_field")
    def test_full_clean_on_null_value(self):
        instance = JSONFieldModel.objects.create(data=None)  # data = SQL Null value
        instance.full_clean()

tests/runtests.py json_null_tests                    
======================================================================
ERROR: test_full_clean_on_null_value (json_null_tests.tests.CheckConstraintTests.test_full_clean_on_null_value)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/olivier/dev/django.dev/olibook/django/django/test/testcases.py", line 1428, in skip_wrapper
    return test_func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/olivier/dev/django.dev/olibook/django/tests/json_null_tests/tests.py", line 13, in test_full_clean_on_null_value
    instance.full_clean()
  File "/Users/olivier/dev/django.dev/olibook/django/django/db/models/base.py", line 1504, in full_clean
    raise ValidationError(errors)
django.core.exceptions.ValidationError: {'__all__': ['Constraint “json_data_cant_be_json_null” is violated.']}

----------------------------------------------------------------------

Attached a patch that will create run json_null_tests folder in django's tests directories. Test can be run with

tests/runtests.py json_null_tests

NOTE : in django 5.1 the CheckConstraint.check parameter as been renamed to CheckConstraint.condition.

I ran a git bisect on this test case:

8fcb9f1f106cf60d953d88aeaa412cc625c60029 is bad
e14d08cd894e9d91cb5d9f44ba7532c1a223f458 is good

git bisect run tests/runtests.py json_null_tests

5c23d9f0c32f166c81ecb6f3f01d5077a6084318 is identified as first bad commit

the issue appears with both sqlite and postgres backends

A few words on what we I'm trying to achieve (some context on the regression):

The json_data_cant_be_json_null aims to prevent having JsonModel.data = None (SQL Null) and JSONModel.data = Value(None, JSONField()) (JSON null) values in the database. These leads to unwanted behaviors when filtering on the null value, and I settled on representing the absence of value with SQL Null and not JSON null.

The app was running in django 4.1 and I'm upgrading it to later django version. That's how the issue appeared.

Attachments (1)

json_null_tests.patch (1.3 KB ) - added by Olivier Tabone 13 days ago.

Download all attachments as: .zip

Change History (3)

by Olivier Tabone, 13 days ago

Attachment: json_null_tests.patch added

comment:1 by Natalia Bidart, 13 days ago

Cc: David Sanders Simon Charette Mariusz Felisiak added
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

I think this is related to #34754, and I originally thought that this was another consequence of what Simon said there: I think the fundamental problem here is that:

MyModel.objects.create(values=None)  # Creates an object with a SQL NULL
MyModel.objects.filter(values=None)  # Filters for objects with JSON NULL

but then I ran the proposed test with --debug-sql (and postgres) and noticed that the check code is casting the value in the database to ::JSONB independently of whether the value is None or not:

INSERT INTO "ticket_35381_jsonfieldmodel" ("data")
VALUES (NULL) RETURNING "ticket_35381_jsonfieldmodel"."id";

args=(NONE,);

ALIAS=DEFAULT (0.000)
SELECT 1 AS "_check"
WHERE COALESCE((NOT ('null'::JSONB = 'null'::JSONB)), TRUE);

args=(Int4(1),
      Jsonb(NONE),
      Jsonb(NONE),
      TRUE);

ALIAS=DEFAULT

So I kept digging and bisected the "bad" commit to 5c23d9f0c32f166c81ecb6f3f01d5077a6084318, and something like this makes the test pass:

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

    diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py
    index 1b219e620c..9bd6c4b252 100644
    a b class JSONField(CheckFieldDefaultMixin, Field):  
    103103            value.output_field, JSONField
    104104        ):
    105105            value = value.value
    106         elif hasattr(value, "as_sql"):
     106        elif value is None or hasattr(value, "as_sql"):
    107107            return value
    108108        return connection.ops.adapt_json_value(value, self.encoder)

it makes the test model_fields.test_jsonfield.TestSaveLoad.test_json_null_different_from_sql_null fails for the check on the query:

NullableJSONModel.objects.filter(value=Value(None, JSONField())) ==  [json_null]

So further investigation is needed to come up with an acceptable solution.

Last edited 13 days ago by Natalia Bidart (previous) (diff)

comment:2 by Natalia Bidart, 13 days ago

Ticket #35167 is related but I don't think is a dupe.

Last edited 13 days ago by Natalia Bidart (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top