#35381: Provide `JSONNull` expression to represent JSON `null` value
-------------------------------------+-------------------------------------
     Reporter:  Olivier Tabone       |                    Owner:  Clifford
                                     |  Gama
         Type:  New feature          |                   Status:  assigned
    Component:  Database layer       |                  Version:  dev
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Jacob Walls:

Old description:

> From Simon Charette in comment:3, in reference to the issues between
> SQL's `NULLL` and JSON's `null`:
>
> In order to finally solve this null ambiguity problem we should:
>
> 1. Introduce a `JSONNull` expression to disambiguate between the two. It
> would also allow the creation of model instances with a JSON `null` which
> is convoluted today as it requires `models.Value(None,
> models.JSONField())` to be used.
> 2. Deprecate `filter(jsonfield=None)` meaning JSON `null` by requiring
> `JSONNull` to be used instead. Should we only do this at the top level to
> still allow `jsonfield__key=None` to filter against null keys? An
> alternative would be to introduce a `__jsonnull` lookup.
>
> The good news is that Django makes it very hard to store JSON `null` in
> the first place since #34754 so this kind of constraints should be rarely
> needed.
>
> == Old 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.

New description:

 From Simon Charette in comment:3, in reference to the issues between SQL's
 `NULL` and JSON's `null`:

 In order to finally solve this null ambiguity problem we should:

 1. Introduce a `JSONNull` expression to disambiguate between the two. It
 would also allow the creation of model instances with a JSON `null` which
 is convoluted today as it requires `models.Value(None,
 models.JSONField())` to be used.
 2. Deprecate `filter(jsonfield=None)` meaning JSON `null` by requiring
 `JSONNull` to be used instead. Should we only do this at the top level to
 still allow `jsonfield__key=None` to filter against null keys? An
 alternative would be to introduce a `__jsonnull` lookup.

 The good news is that Django makes it very hard to store JSON `null` in
 the first place since #34754 so this kind of constraints should be rarely
 needed.

 == Old 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.

--
-- 
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:31>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/0107019a26926af0-f2582fa8-e458-49a7-a55d-1fcaa3ddea99-000000%40eu-central-1.amazonses.com.

Reply via email to