Opened 3 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#37348 closed Bug (fixed)

Removing null=True from GeneratedField should be a SQL no-op

Reported by: Michal Porteš Owned by: Natalia Bidart
Component: Database layer (models, ORM) Version: 6.1
Severity: Release blocker Keywords:
Cc: Dai-Tado, Nilesh Pahari Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As of Django 6.1, having a GeneratedField declared with an explicit null=True causes the warning (fields.W225) null has no effect on GeneratedField.

Consider the following minimal example:

class NumericPrefix(models.Func):
    function = "substring"
    template = r"%(function)s(%(expressions)s from '^\d+')"

class Foo(models.Model):
    code = models.CharField(max_length=50)
    code_num_prefix = models.GeneratedField(
        expression=NumericPrefix("code"),
        output_field=models.CharField(max_length=50),
        db_persist=True,
        null=True,
    )

When I try to resolve the warning by removing null=True, the following happens:

  • the makemigrations command prompts the dialog
It is impossible to change a nullable field 'code_num_prefix' on foo to non-nullable without providing a default. This is because the database needs something to populate existing rows.
Please select a fix:
 1) Provide a one-off default now (will be set on all existing rows with a null value for this column)
 2) Ignore for now. Existing rows that contain NULL values will have to be handled manually, for example with a RunPython or RunSQL operation.
 3) Quit and manually define a default value in models.py.

I select 2 because my table contains NULL values that I don’t want to change.

  • sqlmigrate of the new migration outputs the following SQL:
BEGIN;
--
-- Alter field code_num_prefix on foo
--
ALTER TABLE "foo_foo" ALTER COLUMN "code_num_prefix" SET NOT NULL;
COMMIT;
  • attempting to run migrate fails with
django.db.utils.IntegrityError: column "code_num_prefix" of relation "foo_foo" contains null values

(as mentioned above — my table contains NULL values)

These are all familiar steps when altering a normal column from nullable to non-nullable. But in this case it's unexpected and inconsistent with the warning's message.

​This example can be used to demonstrate that the DDL for the generated field is the same regardless of whether null=True is passed or not. Therefore I believe that this might be a bug and that removing null=True from GeneratedField should be a no-op at the database level.

Change History (18)

comment:1 by Sarah Boyce, 3 weeks ago

Severity: Normal → Release blocker
Summary: Removing `null=True` from GeneratedField should be a SQL no-op → Removing null=True from GeneratedField should be a SQL no-op
Triage Stage: Unreviewed → Accepted

Thank you for the report!
I think ideally there would be no migration at all
I am also wondering if #36806 should be mentioned in the 6.1 release notes

comment:2 by Md. Saikat Islam, 3 weeks ago

Owner: set to Md. Saikat Islam
Status: new → assigned

comment:3 by Natalia Bidart, 3 weeks ago

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

    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.

comment:4 by Natalia Bidart, 3 weeks ago

DB Backend Specifics

DB Virtual Stored NOT NULL allowed Effect of the null=True removal migration
PostgreSQL 18+ 12+ ALTER COLUMN ... SET NOT NULL; fails if NULLs exist
MySQL Yes Yes MODIFY without the AS clause is read as converting to a plain column: stored fails on NULLs ("Data truncated"), virtual errors ("Changing the STORED status is not supported")
MariaDB Yes Yes Same MODIFY: stored fails on NULLs ("Data truncated"), virtual errors ("This is not yet supported for generated columns")
SQLite 3.31+ 3.31+ Table rebuild ignores null; no-op
Oracle Yes 23.7+ Not run yet

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.
Last edited 3 weeks ago by Natalia Bidart (previous) (diff)

comment:5 by Simon Charette, 3 weeks ago

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

Strong agree here, the system check seems wrong as Field.null is used to inform 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)

Last edited 3 weeks ago by Simon Charette (previous) (diff)

comment:6 by Natalia Bidart, 3 weeks ago

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

comment:7 by Natalia Bidart, 3 weeks ago

Owner: changed from Md. Saikat Islam to Natalia Bidart

comment:8 by Natalia Bidart, 3 weeks ago

Has patch: set

in reply to:  6 ; comment:9 by Jacob Walls, 3 weeks ago

Super, let's land the revert of the warning today.


Replying to 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 Natalia Bidart:

the MySQL/MariaDB MODIFY issue

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

        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.

comment:10 by Jacob Walls, 3 weeks ago

Triage Stage: Accepted → Ready for checkin

Revert LGTM, I left only minor feedback about the docs, don't need to re-review.

comment:11 by Jacob Walls, 3 weeks ago

Cc: Dai-Tado Nilesh Pahari added

in reply to:  9 comment:12 by Jacob Walls, 3 weeks ago

Replying to 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!

comment:13 by nessita <124304+nessita@…>, 3 weeks ago

In e7f464e:

Refs #37348 -- Reverted "Refs #36806 -- Removed unnecessary null=True from GeneratedField in test models.".

This reverts commit 968f3f96373e028f1486d135e38331fcd0e3a0ca.

comment:14 by nessita <124304+nessita@…>, 3 weeks ago

Resolution: → fixed
Status: assigned → closed

In fb32a56:

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.

comment:15 by nessita <124304+nessita@…>, 3 weeks ago

In 2abf9d2c:

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.

comment:16 by Natalia <124304+nessita@…>, 3 weeks ago

In 2131ee9:

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

comment:17 by Natalia <124304+nessita@…>, 3 weeks ago

In d8b332c:

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

comment:18 by Natalia <124304+nessita@…>, 3 weeks ago

In 47ee9d03:

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

Note: See TracTickets for help on using tickets.
Back to Top