NOT_NULL validation incorrect with sqlite database

[duncanmorris]

To make creating a dev environment as easy as possible we use sqlite, but this causes issues with the linter, where it falsely warns about NOT_NULL

I think this is due to the issue identified in this comment , but that issue has now been closed so I thought a new issue was appropriate.

If I add the following to an existing model

linter_example_field = models.CharField(
    verbose_name="New Field to test migration linter",
    max_length=100,
    null=True,
    blank=True,
    default="default value",
)

The the linter wrongly identifies this as an invalid migrations

❯ python manage.py makemigrations accounts
Migrations for 'accounts':
  apps/accounts/migrations/0122_auto_20210303_1224.py
    - Add field linter_example_field to section
Linting for 'accounts':
(accounts, 0122_auto_20210303_1224)... ERR
	NOT NULL constraint on columns

The migration linter detected that this migration is not be backward compatible.
- If you keep the migration, you will want to fix the issue or ignore the migration.
- By default, the newly created migration file will be deleted.
Do you want to keep the migration? [y/N] y

I believe this is caused by this line which just checks for NOT NULL.

I've put the SQL below that shows that the sqlite backend implements the new column by creating a new table (that happens to have a NOT NULL field (in this case the PRIMARY KEY)

The sql generated is:

python manage.py sqlmigrate accounts 0122_auto_20210303_1224
BEGIN;
--
-- Add field linter_example_field to section
--
CREATE TABLE "new__accounts_section" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, ...snip..., "linter_example_field" varchar(100) NULL);
INSERT INTO "new__accounts_section" ("id", ...snip..., "linter_example_field") SELECT "id", ...snip..., 'default value' FROM "accounts_section";
DROP TABLE "accounts_section";
ALTER TABLE "new__accounts_section" RENAME TO "accounts_section";
COMMIT;

[David-Wobrock]

Hi @duncanmorris
First, thanks for submitting an issue and taking interest in the project :)

The linked issue is indeed closely related to what you are reporting.
Mainly, the fact that the migration copies the existing data into a new table and drops the old table for sqlite makes it quite hard to detect the changes solely from the SQL. Since you only have the schema of the new table, one can't diff with the existing table, since the linter only uses the generated SQL to detect changes currently.

So it's basically a "shortcut" for now to consider it invalid since the linter should generally prefer to be pessimistic and rather have false positive than omiting a potential dangerous case.

An quick and dirty fix could be adding to add a warning in these cases for sqlite.
And the nice fix, but much harder, could be to introduce a schema/model introspection before linting the generated migration.

[vanschelven]

An quick and dirty fix could be adding to add a warning in these cases for sqlite.

I would appreciate that fix