Re: [Django] #35381: Provide `JSONNull` expression to represent JSON `null` value

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Clifford Gama):

* owner: (none) => Clifford Gama
* status: new => assigned

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

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

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

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Another edge case of the dual interpretation of `None`
[https://docs.djangoproject.com/en/5.2/topics/db/queries/#storing-and-
querying-for-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

{{{#!python
Foo.objects.get_or_create(bar=123, json_field=None)
}}}

will perform

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

using `JSONNull()` 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:17>

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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`."
--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:18>

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

A wrinkle is that `__exact=None` is pretty
[https://docs.djangoproject.com/en/5.2/ref/models/querysets/#exact 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:19>

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

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

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

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

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

I share your position 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:22>

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

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

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sage Abdullah):

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

Replying to [comment:18 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:24>

[Django]

#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: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Clifford Gama):

Thank you all for clarifying things
--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:25>

[Django]

#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: Accepted

Has patch: 1 | Needs documentation: 0

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

Changes (by Clifford Gama):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35381#comment:26>