Provide `JSONNull` expression to represent JSON `null` value

[Olivier Tabone]

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.

[Natalia Bidart]

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 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

@@ -103 +103 @@
             value.output_field, JSONField
         ):
             value = value.value
-        elif hasattr(value, "as_sql"):
+        elif value is None or hasattr(value, "as_sql"):
             return value
         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.

[Natalia Bidart]

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

[Simon Charette]

I've looked into it as well and #35167 only provides a partial solution here IMO as it would require the Q.check logic to also pass around for_save to activate the code path it introduces.

I'm starting to believe that in order to finally solve this null ambiguity problem we should

1. Introduce a JSONNull expression to disambigutate between the two. It would also allow the creation of model instances with a JSON null which is convoluated 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.

[Natalia Bidart]

Thank you Simon. In my view, and in an ideal situation, we shouldn't allow "anywhere" to use None as the json's null. I think we should reserve None only to represent SQL's NULL, and have a representation for the (string) json's null (I understand this matches the new JSONNull expression you are proposing).

Should we only do this at the top level to still allow jsonfield__key=None to filter against null keys?

I would say no, because it then gets very confusing.

An alternative would be to introduce a __jsonnull lookup.

Would this lookup allow further lookups to be chained?

[Simon Charette]

I think we should reserve None only to represent SQL's NULL, and have a representation for the (string) json's null (I understand this matches the new JSONNull expression you are proposing).

Yes it matches the JSONNull proposal. If we take away the ability to use None for filtering against JSON null we should provide an alternative.

The deprecation could look like

1. Introduce JSONNull
2. Make filter(jsonfield=None) raise a deprecation warning pointing at either using filter(jsonfield__isnull=True) or filter(jsonfield=JSONNull)
3. At the end of the deprecation period switch filter(jsonfield=None) to mean filter(jsonfield__isnull=True) like on all other fields

It leaves the problem of having JSON null not surviving a round trip to the database as both SQL NULL and json.loads("null") are turned into Python None but that's a different issue that can be addressed with a specialized decoder if users require it.

Would this lookup allow further lookups to be chained?

It could or not, what gets returned from a lookup can be prevent further chaining if we want.

[Mohammad Salehi]

Hello, if you're okay with it, I would like to start working on this ticket and submit a PR to address the issue.

[David Wobrock]

Replying to Mohammad Salehi:

Hello, if you're okay with it, I would like to start working on this ticket and submit a PR to address the issue.

Of course! Feel free to assign the ticket to you and open a patch.
Bear in mind the suggested approach from the discussions above :)

[Natalia Bidart]

In the context of investigating #36418, I have re-reviewed the different tickets that we have related to confusing behaviours of SQL's NULL vs JSON's string null, and I think this ticket should be edited in summary and description to match the New Feature described in comment:5 (or alternatively, close this one as invalid and open a new ticket).

[Jacob Walls]

#36508 was a dupe. We'll want to highlight this deprecation prominently in the release notes, since unlike top-level JSON nulls, JSON nulls in key/value pairs can be expected to appear more frequently.

[Simon Charette]

Another edge case of the dual interpretation of None ​when storing and querying JSONField (SQL NULL on persisting and filter(json_field=None) on filtering) that came up at work today is how it breaks get_or_create expectations that the filter and creation value will match.

It means that doing

Foo.objects.get_or_create(bar=123, json_field=None)

will perform

SELECT * FROM foo WHERE bar = 123 AND json_field = 'null'::jsonb
INSERT INTO foo (bar, json_field) VALUES (123, NULL) 

using JSONNull() (or it's closet equivalent today being Value(None, JSONField())) would allows for JSON null to be used in both cases (querying and storing) but there are no ways to specify that SQL NULL must be used in both cases which is a supporting point for deprecating filter(json_field=None) meaning JSON null. Here is ​an attempt at getting it IS NULL to be used for comparison and persistence.

[Clifford Gama]

I’d like clarification on whether we also want to deprecate filter(data__key=None).

As I understand it, the ambiguity motivating deprecation at the top level does not exist inside a JSON document. A key’s value can be JSON null, but it cannot be SQL NULL. For that reason, I think .filter(data__key=None) should continue to match JSON null.

We could still allow .filter(data__key=JSONNull()) for users who want explicitness, without deprecating None usage in this case, especially as Jacob pointed out, this case is more common. Additionally, on the creation side, supporting nested JSON null like {"key": JSONNull()} in inserts/updates would require a custom encoder, which isn’t the case for JSONNull() at the top level. Since users can already override encoders/decoders, we can’t make that work universally. We could bypass this problem by introducing and promoting (in the docs) the use of __is_jsonnull lookup for querying and JSONNull() for creating top-level JSON null.

If we don’t deprecate using None in KeyTransform lookups to mean JSON null, then ticket #36508 would have to be reopened, as it is surprising that (jsonfield__key__iexact=None) is translated to an __isnull lookup when jsonfield__key=None means "does key have a value of JSON null."

[Jacob Walls]

A wrinkle is that __exact=None is pretty ​clearly documented to be synonymous with __isnull=True, so by not deprecating we'd be choosing to memorialize something I found as a user to be a "gotcha".

Additionally, on the creation side, supporting nested JSON null like {"key": JSONNull()} in inserts/updates would require a custom encoder, which isn’t the case for JSONNull() at the top level.

Would you mind fleshing this out slightly, to make sure I'm catching your drift?

[Clifford Gama]

A wrinkle is that __exact=None is pretty ​clearly documented to be synonymous with __isnull=True, so by not deprecating we'd be choosing to memorialize something I found as a user to be a "gotcha".

Makes sense.

Would you mind fleshing this out slightly, to make sure I'm catching your drift?

I meant that doing Dog(data={"name": JSONNull()}).save()} would probably require us to build a custom encoder since json.dumps() will not recognise the JSONNull() expression.

[Jacob Walls]

I meant that doing Dog(data={"name": JSONNull()}).save()} would probably require us to build a custom encoder since json.dumps() will not recognise the JSONNull() expression.

Thanks. Simon discussed this concern for top-level JSON null in comment:5:

It leaves the problem of having JSON null not surviving a round trip to the database as both SQL NULL and json.loads("null") are turned into Python None but that's a different issue that can be addressed with a specialized decoder if users require it.

... which I think is an acceptable trade-off for top-level JSON null.

For nulls in key/value pairs, as in your example, I think it's still an acceptable trade-off, as long as we are not *removing* the ability of users to store python None in key/value pairs. The plan I see from the above comments is just to deprecate & change the behavior of filtering on None in key/value pairs. (EDIT: not really, see comment:22 and comment:23 for updates.)

[Simon Charette]

I share your position Clifford and Jacob.

To me the ambiguity of what to do with None only exists for top level values of JSONField(null=True); there's no ambiguity for key values as it always mean JSON null.

To make it clear I also think that filter(data__key=None) should continue to match JSON null and I'm not opposed to supporting filter(data__key=JSONNull()) while I don't see much benefits to it.

[Jacob Walls]

Oh this clarifies things for me. I'll edit my recent comments to avoid suggesting there was consensus to change the behavior of filtering on None in key values, since it sounds like that was only proposed in comment:4, and later as Clifford points out, in the triage decision on #36508, but now we have multiple people with misgivings about doing that.

Having just left a project that managed most of its data in JSONFields, I can offer that auditing the behavior of key=var lookups everywhere to figure out what's intended when var is possibly None would be a daunting expense.

Re: the "gotcha" of exact=None and isnull=True not being invariant, we can just doc it clearly.

[Sage Abdullah]

Thanks all, I was just about to post what Simon said.

Replying to Clifford Gama:

If we don’t deprecate using None in KeyTransform lookups to mean JSON null, then ticket #36508 would have to be reopened, as it is surprising that (jsonfield__key__iexact=None) is translated to an __isnull lookup when jsonfield__key=None means "does key have a value of JSON null."

I agree, that ticket is a separate issue and I think it should be reopened. I'll leave a comment there for more details.

[Clifford Gama]

Thank you all for clarifying things