Opened 3 months ago

Closed 3 months ago

#35006 closed Bug (fixed)

migrate crashes with db_table_comment added to existing model on SQLite

Reported by: Юрий Owned by: Mariusz Felisiak
Component: Migrations Version: 4.2
Severity: Release blocker Keywords: sqlite migrations
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Юрий)

It seems that I have found a bug in all 4.2+ versions of Django (including main branch and 5.0rc). I made a repo with steps to reproduce and full description of the bug – https://github.com/FeroxTL/django-sqlite-bug . Long story short: sqlite does not support comments, but when you change (not create) Meta.db_table_comment sqlite backend django.db.backends.sqlite3.schema.DatabaseSchemaEditor tries to generate COMMENT sql and it obviously falls.

$./manage.py migrate
System check identified some issues:

WARNINGS:
question.QuestionThree: (models.W046) SQLite does not support comments on tables (db_table_comment).
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, question, sessions
Running migrations:
  Applying question.0001_initial... OK
  Applying question.0002_alter_questionthree_table_comment...Traceback (most recent call last):
  File "env/lib/python3.10/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
  File "env/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 328, in execute
    return super().execute(query, params)
sqlite3.OperationalError: near "COMMENT": syntax error

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "proj/./manage.py", line 22, in <module>
    main()
  File "proj/./manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "env/lib/python3.10/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "env/lib/python3.10/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "env/lib/python3.10/site-packages/django/core/management/base.py", line 412, in run_from_argv
    self.execute(*args, **cmd_options)
  File "env/lib/python3.10/site-packages/django/core/management/base.py", line 458, in execute
    output = self.handle(*args, **options)
  File "env/lib/python3.10/site-packages/django/core/management/base.py", line 106, in wrapper
    res = handle_func(*args, **kwargs)
  File "env/lib/python3.10/site-packages/django/core/management/commands/migrate.py", line 356, in handle
    post_migrate_state = executor.migrate(
  File "env/lib/python3.10/site-packages/django/db/migrations/executor.py", line 135, in migrate
    state = self._migrate_all_forwards(
  File "env/lib/python3.10/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
    state = self.apply_migration(
  File "env/lib/python3.10/site-packages/django/db/migrations/executor.py", line 252, in apply_migration
    state = migration.apply(state, schema_editor)
  File "env/lib/python3.10/site-packages/django/db/migrations/migration.py", line 132, in apply
    operation.database_forwards(
  File "env/lib/python3.10/site-packages/django/db/migrations/operations/models.py", line 610, in database_forwards
    schema_editor.alter_db_table_comment(
  File "env/lib/python3.10/site-packages/django/db/backends/base/schema.py", line 640, in alter_db_table_comment
    self.execute(
  File "env/lib/python3.10/site-packages/django/db/backends/base/schema.py", line 201, in execute
    cursor.execute(sql, params)
  File "env/lib/python3.10/site-packages/django/db/backends/utils.py", line 102, in execute
    return super().execute(sql, params)
  File "env/lib/python3.10/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
  File "env/lib/python3.10/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "env/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "env/lib/python3.10/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "env/lib/python3.10/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
  File "env/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 328, in execute
    return super().execute(query, params)
django.db.utils.OperationalError: near "COMMENT": syntax error

Change History (6)

comment:1 by Юрий, 3 months ago

Description: modified (diff)

comment:2 by David Sanders, 3 months ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks for the report 🏆

comment:3 by David Sanders, 3 months ago

Summary: Django db_table_comment with sqlite databasemigrate crashes with db_table_comment added to existing model on SQLite

The simple fix is to add the supports_comments condition to AlterModelTableComment.database_forwards() [1]

A better solution may be to use hooks like we've been doing recently so that the "supports" decision is centralised and won't be inadvertently left out anywhere.

[1] https://github.com/django/django/blob/06c5cb1284557edc05f963afb6f6bcdd55b00911/django/db/migrations/operations/models.py#L581

Last edited 3 months ago by David Sanders (previous) (diff)

comment:4 by Mariusz Felisiak, 3 months ago

Owner: changed from nobody to Mariusz Felisiak
Status: newassigned
Version: dev4.2

comment:5 by Mariusz Felisiak, 3 months ago

Has patch: set

comment:6 by GitHub <noreply@…>, 3 months ago

Resolution: fixed
Status: assignedclosed

In 37fc832:

Fixed #35006 -- Fixed migrations crash when altering Meta.db_table_comment on SQLite.

Thanks Юрий for the report.

Regression in 78f163a4fb3937aca2e71786fbdd51a0ef39629e.

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