Re: [Django] #37348: Removing null=True from GeneratedField should be a SQL no-op

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Md.
| Saikat Islam
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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 Md. Saikat Islam):

* owner: (none) => Md. Saikat Islam
* status: new => assigned

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

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Md.
| Saikat Islam
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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 Natalia Bidart):

I looked into this and I think the problem is bigger than the (new)
migration setting the column to `NOT NULL`.

`null` is **not** a no-op for `GeneratedField` at the ORM level:
`Query.is_nullable()` reads `field.null`, and negated lookups use it to
add the `IS NOT NULL` guard. Without `null=True`,
`Foo.objects.exclude(code_num_prefix="12")` compiles to `NOT (col =
'12')`, so rows where the generated column is `NULL` are silently dropped.
So if we follow the `W225` advice, we change query results, independently
of the migration issue reported here.

This can be reproduced with the existing `GeneratedModelNull` test model
(whose `null=True` was removed in 968f3f9637 to silence W225) and this
test, which fails (at least) in SQLlite and Postgres, for both the stored
and virtual variants (m1 is missing).

{{{#!python
def test_nullable_exclude(self):
m1 = self.nullable_model.objects.create()
m2 = self.nullable_model.objects.create(name="NaMe")
self.nullable_model.objects.create(name="Other")
self.assertSequenceEqual(
self.nullable_model.objects.exclude(lower_name="other").order_by("pk"),
[m1, m2],
)
}}}

The migration side-effect comes from the fact that `null` is still stored
and deconstructed, so the autodetector sees a change, and `_alter_field()`
emits `SET NOT NULL` even though column creation ignores null for
generated fields.

I agree this is a release blocker for 6.1.x, and my suggestion is to
revert #36806. Then, for main/6.2, we could consider to treat
`GeneratedField` as always nullable (force `null=True` and drop it from
`deconstruct()`), since nullability depends on the expression and
database. That would make removing `null=True` a no-op both for migrations
and queries.

I'll post another comment with some robot'd research on backend specifics.
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:3>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Md.
| Saikat Islam
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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 Natalia Bidart):

== DB Backend Specifics ==

||= DB =||= Virtual =||= Stored =||= NOT NULL allowed =||= Effect of the
null=True removal migration =||
|| PostgreSQL || 18+ || 12+ || Stored: yes; virtual: to verify || Adds
constraint; fails if NULLs exist ||
|| MySQL || Yes || Yes || Yes || `MODIFY` without `AS` clause: may convert
a stored column to a plain column (to verify) ||
|| MariaDB || Yes || Yes || Historically no (to verify) || Likely error ||
|| SQLite || 3.31+ || 3.31+ || Yes || Table rebuild; effectively a no-op
||
|| Oracle || Yes || 23.7+ || Yes || Adds constraint; fails if NULLs exist
||

The exclude bug affects all backends, except string outputs on Oracle.
This backend spread supports the "always nullable" approach. Nobody can
know nullability statically, Django doesn't emit NOT NULL on creation
anywhere, and the alter path gives different results on each backend. If
someone really wants NOT NULL, a CheckConstraint is the portable way to
get it.

== Longer explanation (Claude Opus 5) ==
Here's how each backend handles nullability of generated columns. The
Django parts come from the repo; the database behavior is from my
knowledge of each vendor's docs, which I haven't tested. I've marked the
claims worth double-checking before you rely on them in the ticket.

=== The common ground ===
No backend infers NOT NULL from the expression. A generated column is
nullable unless you add a NOT NULL constraint. Whether it can actually
hold NULL depends on the expression (Lower("name") over a nullable column
can) and sometimes on the backend (e.g. how string functions treat NULL).
That is why "null has no effect" is true for the DDL Django emits but
false as a statement about the data.

Django never emits NOT NULL for a generated column when creating a table
(base/schema.py:362). The only way it adds NOT NULL is the _alter_field
path from this ticket, and that path behaves differently per backend.

=== PostgreSQL (Django minimum is 15) ===
- Stored generated columns exist since PG 12. Virtual ones were added in
PG 18, which also made virtual the default when neither is specified.
Django gates this with supports_virtual_generated_columns =
is_postgresql_18.
- NOT NULL on a stored generated column is allowed, and ALTER COLUMN ...
SET NOT NULL works; it fails if existing rows are NULL, as the reporter
saw.
- For virtual columns on PG 18, I believe NOT NULL and CHECK constraints
are allowed while unique constraints, indexes and foreign keys are not.
Verify that before citing it.
- Result: the migration from the ticket really adds a constraint that a
fresh CREATE TABLE would not have. Databases migrated from older
migrations end up with a different schema than new ones.

=== MySQL (8.4+) and MariaDB (10.11+) ===
- Both support stored and virtual columns.
- MySQL allows NOT NULL in a generated column definition.
- MariaDB historically did not allow NOT NULL on generated columns. Verify
for 10.11+.
- Django's MySQL schema editor uses sql_alter_column_not_null = "MODIFY
%(column)s %(type)s NOT NULL". That statement omits the AS (...) clause.
MySQL's docs say a stored generated column can be converted to a regular
column with MODIFY/CHANGE, and a virtual one cannot. So on MySQL, this
migration may do more than add NOT NULL:
- If the table has NULLs, it fails.
- If it doesn't, a stored generated column could silently become a plain
column.
- A virtual column should just error.

This is the most serious case, but it needs an actual test on
MySQL/MariaDB before it goes in the ticket.

=== SQLite (3.37+) ===
- Generated columns exist since 3.31, and both kinds are supported. NOT
NULL and CHECK constraints are allowed; DEFAULT and PRIMARY KEY are not.
- Django's _alter_field on SQLite rebuilds the table, and the new column
definition goes through the column SQL that ignores null for generated
fields. So the reporter's migration is in effect a no-op on SQLite: the
table is rebuilt without NOT NULL. That explains why this goes unnoticed
in a SQLite-only test run.

=== Oracle (19+) ===
- Oracle has had virtual columns since 11g. Stored ("materialized")
generated columns need 23.7+ in Django
(supports_stored_generated_columns).
- NOT NULL constraints on virtual columns are allowed. Django would emit
MODIFY col NOT NULL, which should work and fails if NULLs exist.
- Oracle treats `''` as NULL (interprets_empty_strings_as_nulls), so
is_nullable() returns True for string outputs regardless of null. The
exclude bug is hidden there for CharField/TextField outputs, but still
shows for e.g. IntegerField outputs.
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:4>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Md.
| Saikat Islam
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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):

> null is not a no-op for `GeneratedField` at the ORM level

Strong agree here, the system check seems wrong as it's used for exclusion
and JOIN promotion.

It can also be useful to have a generated column marked as `NOT NULL` at
the database level as it can inform the plan used by the database.

The ORM still lacks a way to automatically resolve nullability when mixing
expressions of different types (e.g. `_resolve_output_field` should carry
over `null=True` on combination) and a mechanism to palliate to that is to
allow an explicit `output_field` to be specified. If we take that away
from users we prevent them from expressing types that are effectively
nullable.

I think we should consider reverting #36806
(6025eab3c509b4de922117e16866bbfe0ee99aa6)
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:5>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Md.
| Saikat Islam
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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 Natalia Bidart):

[https://github.com/django/django/pull/21979 Exploratory PR with some
tests to be run on every DB backend] (now closed) confirms both problems:

* `exclude()` on a generated field without `null=True` drops rows where
the generated value is NULL: fails on SQLite, PostgreSQL, MySQL, and
MariaDB.
* Altering a generated field from `null=True` to not null behaves
differently on every backend:

Results:
* PostgreSQL: `ALTER COLUMN ... SET NOT NULL`, fails if NULLs exist
* MySQL: `MODIFY` without the `AS` clause, read as converting to a regular
column: stored fails on NULLs, virtual errors
* MariaDB: Same as MySQL
* SQLite: Table rebuild ignores `null`, no-op
* Oracle: No-op: `GeneratedField` inherits `empty_strings_allowed = True`
from `Field` regardless of `output_field`, so Oracle treats it as always
nullable (virtual tested on 19; stored not run)

I agree with Simon, I think we need to revert #36806, I will propose a PR.
The CREATE vs ALTER inconsistency and the MySQL/MariaDB `MODIFY` issue
predate #36806 and deserve separate tickets IMHO.
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:6>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------

Reporter: Michal Porteš | Owner: Natalia
| Bidart

Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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 Natalia Bidart):

* owner: Md. Saikat Islam => Natalia Bidart

--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:7>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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 Natalia Bidart):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/21983 PR with the revert.]
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:8>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

Super, let's land the revert of the warning today.
----
Replying to [comment:6 Natalia Bidart]:

> The CREATE vs ALTER inconsistency and the MySQL/MariaDB `MODIFY` issue
predate #36806 and deserve separate tickets IMHO.

+1 on a new ticket for ''CREATE doesn't act like ALTER for placing NOT
NULL in the column definition for GeneratedField'', but offhand I don't
have an answer for the backward compatibility issue if we start treating
historical migrations as implying NOT NULL column definitions.
----
Replying to [comment:6 Natalia Bidart]:
> the MySQL/MariaDB `MODIFY` issue

Modifying a `GeneratedField` is supposed to throw an exception before any
migration is created:

{{{#!py
if modifying_generated_field:
raise ValueError(
f"Modifying GeneratedFields is not supported - the field
{new_field} "
"must be removed and re-added with the new definition."
)
}}}

So the fact that a migration creating `NOT NULL` sneaks through and can do
something wacky on MySQL/MariaDB is a bug IMO. There's an `old_sql ==
new_sql` heuristic that is evaluating equal and interpreted as "not
modifying_generated_field" that doesn't match the eventual DDL which adds
a `NOT NULL`.

+1 for a new ticket to fix the incorrect heuristic.
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:9>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* stage: Accepted => Ready for checkin

Comment:

Revert LGTM, I left only minor feedback about the docs, don't need to re-
review.
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:10>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* cc: Dai-Tado, Nilesh Pahari (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:11>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

Replying to [comment:9 Jacob Walls]:

> +1 for a new ticket to fix the incorrect heuristic.

Opened #37351.

> +1 on a new ticket for ''CREATE doesn't act like ALTER ...

Whoops, don't need this one, since the point of #37351 is that you
shouldn't be able to alter at all!
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:12>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart

Type: Bug | Status: closed

Component: Database layer | Version: 6.1
(models, ORM) |

Severity: Release blocker | Resolution: fixed

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

Changes (by nessita <124304+nessita@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"fb32a564ccc6f91d4b7594f5707c62fab0c11c04" fb32a56]:
{{{#!CommitTicketReference repository=""
revision="fb32a564ccc6f91d4b7594f5707c62fab0c11c04"
Fixed #37348 -- Reverted "Fixed #36806 -- Added system check for null
kwarg in GeneratedField.".

This reverts commit 6025eab3c509b4de922117e16866bbfe0ee99aa6, includes a
release note for 6.1.2, and keeps fields.W225 documented as removed.

Thanks to Michal Porteš for the report, and to Jacob Walls and
Simon Charette for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:14>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: closed
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
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
-------------------------------------+-------------------------------------

Comment (by nessita <124304+nessita@…>):

In [changeset:"2abf9d2cf8602f0ddc0db4ec1a33769b41b4232a" 2abf9d2c]:
{{{#!CommitTicketReference repository=""
revision="2abf9d2cf8602f0ddc0db4ec1a33769b41b4232a"
Refs #37348 -- Added test for excluding on a nullable GeneratedField.

Negated lookups rely on `Field.null` to match rows where the value is
`NULL`. This behavior was lost when `null=True` was removed from
`GeneratedField` definitions to silence `fields.W225`.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:15>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart

Type: Bug | Status: assigned

Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | 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
-------------------------------------+-------------------------------------
Comment (by nessita <124304+nessita@…>):

In [changeset:"e7f464ecdc025000899067ceb6bc61116ca81996" e7f464e]:
{{{#!CommitTicketReference repository=""
revision="e7f464ecdc025000899067ceb6bc61116ca81996"
Refs #37348 -- Reverted "Refs #36806 -- Removed unnecessary null=True from
GeneratedField in test models.".

This reverts commit 968f3f96373e028f1486d135e38331fcd0e3a0ca.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:13>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart

Type: Bug | Status: closed

Component: Database layer | Version: 6.1
(models, ORM) |

Severity: Release blocker | Resolution: fixed

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

Comment (by Natalia <124304+nessita@…>):

In [changeset:"2131ee9f198f9f966877db7a5649b4fb93c6bf6e" 2131ee9]:
{{{#!CommitTicketReference repository=""
revision="2131ee9f198f9f966877db7a5649b4fb93c6bf6e"
[6.1.x] Refs #37348 -- Reverted "Refs #36806 -- Removed unnecessary

null=True from GeneratedField in test models.".

This reverts commit 968f3f96373e028f1486d135e38331fcd0e3a0ca.

Backport of e7f464ecdc025000899067ceb6bc61116ca81996 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:16>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: closed
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
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
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):

In [changeset:"d8b332caf17e79da2a9fa3823a2b6e85063dad94" d8b332c]:
{{{#!CommitTicketReference repository=""
revision="d8b332caf17e79da2a9fa3823a2b6e85063dad94"
[6.1.x] Fixed #37348 -- Reverted "Fixed #36806 -- Added system check for

null kwarg in GeneratedField.".

This reverts commit 6025eab3c509b4de922117e16866bbfe0ee99aa6, includes a
release note for 6.1.2, and keeps fields.W225 documented as removed.

Thanks to Michal Porteš for the report, and to Jacob Walls and
Simon Charette for reviews.

Backport of fb32a564ccc6f91d4b7594f5707c62fab0c11c04 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:17>

[Django]

#37348: Removing null=True from GeneratedField should be a SQL no-op
-------------------------------------+-------------------------------------
Reporter: Michal Porteš | Owner: Natalia
| Bidart
Type: Bug | Status: closed
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
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
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):

In [changeset:"47ee9d03094bc8ad4964e584c72908d26085f3b5" 47ee9d03]:
{{{#!CommitTicketReference repository=""
revision="47ee9d03094bc8ad4964e584c72908d26085f3b5"
[6.1.x] Refs #37348 -- Added test for excluding on a nullable

GeneratedField.

Negated lookups rely on `Field.null` to match rows where the value is
`NULL`. This behavior was lost when `null=True` was removed from
`GeneratedField` definitions to silence `fields.W225`.

Backport of 2abf9d2cf8602f0ddc0db4ec1a33769b41b4232a from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37348#comment:18>