Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33899 closed Bug (fixed)

RemoveField on indexed fields crashes on SQLite 3.35.5+

Reported by: cessor Owned by: fizaashraf37
Component: Migrations Version: 4.1
Severity: Release blocker Keywords: Migration Sqlite
Cc: 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 (last modified by 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:

  1. Install django 4.1
  2. Create a new project
  3. Create an app
  4. Install app in project
  5. Create a model
  6. Add field on model, set db_index=True
  7. Make migrations: $ python manage.py makemigrations
  8. Remove field from model
  9. Make migrations: $ python manage.py makemigrations
  10. 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

Attachments (2)

django-bug-33899.tgz (936 bytes ) - added by cessor 2 years ago.
Minimal Example
migrations_33899.zip (6.4 KB ) - added by fizaashraf37 2 years ago.
Regression test of the bug reported

Download all attachments as: .zip

Change History (15)

by cessor, 2 years ago

Attachment: django-bug-33899.tgz added

Minimal Example

comment:1 by cessor, 2 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 2 years ago

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

in reply to:  2 comment:3 by cessor, 2 years ago

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:

by fizaashraf37, 2 years ago

Attachment: migrations_33899.zip added

Regression test of the bug reported

comment:4 by fizaashraf37, 2 years ago

Easy pickings: set
Has patch: set
Needs tests: set
Triage Stage: UnreviewedAccepted

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

comment:5 by cessor, 2 years ago

Description: modified (diff)
Summary: migrations.RemoveField causes OperationalError "no such column" upon migrationmigrations.RemoveField causes OperationalError "no such column" upon migration when db_index=True

comment:6 by cessor, 2 years ago

Description: modified (diff)

comment:7 by fizaashraf37, 2 years ago

Easy pickings: unset
Owner: changed from nobody to fizaashraf37
Status: newassigned

comment:8 by Mariusz Felisiak, 2 years ago

Has patch: unset
Keywords: Docker removed
Needs tests: unset
Severity: NormalRelease blocker
Summary: migrations.RemoveField causes OperationalError "no such column" upon migration when db_index=TrueRemoveField on indexed fields crashes on SQLite 3.35.5+

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

  • django/db/backends/sqlite3/schema.py

    diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py
    index 55fdf5fbfe..6c106ae868 100644
    a b class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):  
    408408            # For explicit "through" M2M fields, do nothing
    409409        elif (
    410410            self.connection.features.can_alter_table_drop_column
    411             # Primary keys, unique fields, and foreign keys are not
    412             # supported in ALTER TABLE DROP COLUMN.
     411            # Primary keys, unique fields, indexed fields, and foreign keys are
     412            # not supported in ALTER TABLE DROP COLUMN.
    413413            and not field.primary_key
    414414            and not field.unique
     415            and not field.db_index
    415416            and not (field.remote_field and field.db_constraint)
    416417        ):
    417418            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.

comment:9 by fizaashraf37, 2 years ago

Has patch: set

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

comment:10 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In c0beff21:

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

Regression in 702819227fd0cdd9b581cd99e11d1561d51cbeb.

Thanks cessor for the report.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

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

comment:13 by cessor, 2 years ago

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

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