RemoveField on indexed fields crashes on SQLite 3.35.5+

[cessor]

Description

I encountered the following error with django 4.1 in my Gitlab CI/CD Pipeline. When I bumped django versions from 4.0.7 to 4.1. my pipeline broke during the testing stage; specifically during db migrations. I have not changed any other source code.

Steps to reproduce

Minimal example attached. Run make green to see that it works with 4.0.7, run make red to see that it does not work with 4.1. It will build and exercise a docker container which installs all dependencies in isolation and sets up an example django app and run migrations.

Manual steps:

0. Install django 4.1
1. Create a new project
2. Create an app
3. Install app in project
4. Create a model
5. Add field on model, set db_index=True
6. Make migrations: $ python manage.py makemigrations
7. Remove field from model
8. Make migrations: $ python manage.py makemigrations
9. Apply migrations: $ python manage.py migrate

The migration fails with the following error (for an app called web, with a model called Entity with a field called attribute for example):

Running migrations:
Applying contenttypes.0001_initial... OK
...
Applying sessions.0001_initial... OK
Applying web.0001_initial... OK
Applying web.0002_remove_entity_attribute...Traceback (most recent call last):
File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 89, in _execute
 return self.cursor.execute(sql, params)
File "/usr/local/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 357, in execute
 return Database.Cursor.execute(self, query, params)
sqlite3.OperationalError: error in index web_entity_attribute_d22c3fcb after drop column: no such column: attribute

Details

The above steps create a set of migrations where at the end a RemoveField migration is produced. Applying this migration fails for fields which had db_index=True. The example I attached uses a SlugField where db_index defaults to True, setting this parameter to False will apply the migration without this error.

I reproduced the error with the following field types: TextField, IntegerField, SlugField, CharField, URLField

[cessor]

Minimal Example

[Mariusz Felisiak]

I'm really puzzled. I cannot reproduce this issue manually, but make red crashes.

[cessor]

Replying to Mariusz Felisiak:

I'm really puzzled. I cannot reproduce this issue manually, but make red crashes.

Thank you for giving this a try. I investigated this some further and realized that my manual description is not precise enough to reproduce the error. The example I attached uses a SlugField, but the error does not occur with other field types, such as CharFields. I noticed that SlugFields set db_index=True by default (See Link 1), and could reproduce the bug with other fields when setting db_index=True. I will change the bug description accordingly.

Links:

• 1: SlugField: ​https://github.com/django/django/blob/main/django/db/models/fields/__init__.py#L2301

[fizaashraf37]

Regression test of the bug reported

[fizaashraf37]

The issue is valid. I have written the regression test of the exact test case. And found the first bad commit as below:

3702819227fd0cdd9b581cd99e11d1561d51cbeb is the first bad commit
commit 3702819227fd0cdd9b581cd99e11d1561d51cbeb
Author: Mariusz Felisiak <felisiak.mariusz@gmail.com>
Date:   Fri Feb 11 22:21:58 2022 +0100

    Refs #32502 -- Avoided table rebuild when removing fields on SQLite 3.35.5+.

    ALTER TABLE ... DROP COLUMN was introduced in SQLite 3.35+ however
    a data corruption issue was fixed in SQLite 3.35.5.

 django/db/backends/sqlite3/features.py |  2 ++
 django/db/backends/sqlite3/schema.py   | 10 ++++++++++
 tests/schema/tests.py                  | 18 ++++++++++++++++++
 3 files changed, 30 insertions(+)
bisect found first bad commit

[Mariusz Felisiak]

Thanks! It should be enough to avoid this optimization on indexes fields:

django/db/backends/sqlite3/schema.py

@@ -408 +408 @@
             # For explicit "through" M2M fields, do nothing
         elif (
             self.connection.features.can_alter_table_drop_column
-            # Primary keys, unique fields, and foreign keys are not
-            # supported in ALTER TABLE DROP COLUMN.
+            # Primary keys, unique fields, indexed fields, and foreign keys are
+            # not supported in ALTER TABLE DROP COLUMN.
             and not field.primary_key
             and not field.unique
+            and not field.db_index
             and not (field.remote_field and field.db_constraint)
         ):
             super().remove_field(model, field)

According to ​SQLite docs:

Possible reasons why the DROP COLUMN command can fail include':'
...

• The column is indexed.

Regression in 3702819227fd0cdd9b581cd99e11d1561d51cbeb.

[fizaashraf37]

Please review the PR and approve workflows if any one of you is the maintainer.
​https://github.com/django/django/pull/15925

[Mariusz Felisiak <felisiak.mariusz@…>]

In c0beff21:

Fixed #33899 -- Fixed migration crash when removing indexed field on SQLite 3.35.5+.

Regression in 702819227fd0cdd9b581cd99e11d1561d51cbeb.

Thanks cessor for the report.

[Mariusz Felisiak <felisiak.mariusz@…>]

In f546e7c:

[4.1.x] Fixed #33899 -- Fixed migration crash when removing indexed field on SQLite 3.35.5+.

Regression in 702819227fd0cdd9b581cd99e11d1561d51cbeb.

Thanks cessor for the report.

Backport of c0beff21239e70cbdcc9597e5be09e505bb8f76c from main

[cessor]

Thank you all for your work and quick responses. Much appreciated!