#34984 closed Bug (fixed)
Adding a field with default crashes for models with GeneratedField on SQLite.
Reported by: | Sarah Boyce | Owned by: | Sarah Boyce |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.0 |
Severity: | Release blocker | Keywords: | 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
I added a non-nullable field (in example new_field_temporary_default
) to a model containing a GeneratedField and in my migration prompt I added a one-off default. I am using SQLite.
Model
class Article(models.Model): created_at = models.DateTimeField(db_default=Now()) title = models.CharField(max_length=300) slug = models.GeneratedField( expression=Lower(Replace(F("title"), Value(" "), Value("-"))), db_persist=True, unique=True, ) new_field_temporary_default = models.TextField() # This field added after table was created
Generated migration:
# Generated by Django 5.0b1 on 2023-11-21 12:52 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ ("blog", "0001_initial"), ] operations = [ migrations.AddField( model_name="article", name="new_field_temporary_default", field=models.TextField(default="Temp default"), preserve_default=False, ), ]
This failed to migrate with the error
return super().execute(query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ django.db.utils.OperationalError: cannot INSERT into generated column "slug"
Not certain if this is an issue. Thought I should raise.
Change History (19)
comment:1 by , 13 months ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 by , 13 months ago
I've upgraded to the release candidate and replicated.
Initially the model looks like this:
class Article(models.Model): created_at = models.DateTimeField(db_default=Now()) title = models.CharField(max_length=300) slug = models.GeneratedField( expression=Lower(Replace(F("title"), Value(" "), Value("-"))), db_persist=True, unique=True, output_field=models.TextField(), )
First migration file:
# Generated by Django 5.0rc1 on 2023-11-21 13:58 import django.db.models.functions.datetime import django.db.models.functions.text from django.db import migrations, models class Migration(migrations.Migration): initial = True dependencies = [] operations = [ migrations.CreateModel( name="Article", fields=[ ( "id", models.BigAutoField( auto_created=True, primary_key=True, serialize=False, verbose_name="ID", ), ), ( "created_at", models.DateTimeField( db_default=django.db.models.functions.datetime.Now() ), ), ("title", models.CharField(max_length=300)), ( "slug", models.GeneratedField( db_persist=True, expression=django.db.models.functions.text.Lower( django.db.models.functions.text.Replace( models.F("title"), models.Value(" "), models.Value("-") ) ), output_field=models.TextField(), unique=True, ), ), ], ), ]
Then I add new_field_temporary_default = models.TextField()
make migrations, it prompts me that this field is non nullable and without a default, I go for the option to add a temporary default
Second migration file
# Generated by Django 5.0rc1 on 2023-11-21 13:58 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ ("blog", "0001_initial"), ] operations = [ migrations.AddField( model_name="article", name="new_field_temporary_default", field=models.TextField(default="Temp value"), preserve_default=False, ), ]
And migrate, same error:
return super().execute(query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ django.db.utils.OperationalError: cannot INSERT into generated column "slug"
comment:3 by , 13 months ago
Thanks Sarah 🏆. Confirmed on main, full traceback:
django-sample % dj migrate Operations to perform: Apply all migrations: admin, auth, contenttypes, htc, query_json_in, sessions, ticket_34984_one_off_default, union_issue Running migrations: Applying ticket_34984_one_off_default.0002_article_new_field_temporary_default...Traceback (most recent call last): File "/path/to/django/django/db/backends/utils.py", line 105, in _execute return self.cursor.execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/backends/sqlite3/base.py", line 328, in execute return super().execute(query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sqlite3.OperationalError: cannot INSERT into generated column "slug" The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/path/to/sample-project/./manage.py", line 22, in <module> main() File "/path/to/sample-project/./manage.py", line 18, in main execute_from_command_line(sys.argv) File "/path/to/django/django/core/management/__init__.py", line 442, in execute_from_command_line utility.execute() File "/path/to/django/django/core/management/__init__.py", line 436, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/path/to/django/django/core/management/base.py", line 412, in run_from_argv self.execute(*args, **cmd_options) File "/path/to/django/django/core/management/base.py", line 458, in execute output = self.handle(*args, **options) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/core/management/base.py", line 106, in wrapper res = handle_func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/core/management/commands/migrate.py", line 356, in handle post_migrate_state = executor.migrate( ^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/migrations/executor.py", line 135, in migrate state = self._migrate_all_forwards( ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/migrations/executor.py", line 167, in _migrate_all_forwards state = self.apply_migration( ^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/migrations/executor.py", line 252, in apply_migration state = migration.apply(state, schema_editor) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/migrations/migration.py", line 132, in apply operation.database_forwards( File "/path/to/django/django/db/migrations/operations/fields.py", line 108, in database_forwards schema_editor.add_field( File "/path/to/django/django/db/backends/sqlite3/schema.py", line 303, in add_field self._remake_table(model, create_field=field) File "/path/to/django/django/db/backends/sqlite3/schema.py", line 233, in _remake_table self.execute( File "/path/to/django/django/db/backends/base/schema.py", line 201, in execute cursor.execute(sql, params) File "/path/to/django/django/db/backends/utils.py", line 122, in execute return super().execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/backends/utils.py", line 79, in execute return self._execute_with_wrappers( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/backends/utils.py", line 92, in _execute_with_wrappers return executor(sql, params, many, context) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/backends/utils.py", line 100, in _execute with self.db.wrap_database_errors: File "/path/to/django/django/db/utils.py", line 91, in __exit__ raise dj_exc_value.with_traceback(traceback) from exc_value File "/path/to/django/django/db/backends/utils.py", line 105, in _execute return self.cursor.execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/django/django/db/backends/sqlite3/base.py", line 328, in execute return super().execute(query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ django.db.utils.OperationalError: cannot INSERT into generated column "slug"
comment:4 by , 13 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:5 by , 13 months ago
Resolution: | worksforme |
---|---|
Status: | closed → new |
comment:6 by , 13 months ago
Thank you Sarah, I tried again and I was able to reproduce, I must have had some misconfiguration in my test project. Thank you!
comment:7 by , 13 months ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Type: | Uncategorized → Bug |
comment:8 by , 13 months ago
Severity: | Normal → Release blocker |
---|
comment:9 by , 13 months ago
Severity: | Release blocker → Normal |
---|
SQL of the migration:
BEGIN; -- -- Add field new_field_temporary_default to article -- CREATE TABLE "new__ticket_34984_article" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "new_field_temporary_default" text NOT NULL, "created_at" datetime DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')) NOT NULL, "title" varchar(300) NOT NULL, "slug" text GENERATED ALWAYS AS (LOWER(REPLACE("title", ' ', '-'))) STORED UNIQUE); INSERT INTO "new__ticket_34984_article" ("id", "created_at", "title", "slug", "new_field_temporary_default") SELECT "id", "created_at", "title", "slug", 'temp' FROM "ticket_34984_article"; DROP TABLE "ticket_34984_article"; ALTER TABLE "new__ticket_34984_article" RENAME TO "ticket_34984_article"; COMMIT;
comment:10 by , 13 months ago
Severity: | Normal → Release blocker |
---|
comment:11 by , 13 months ago
Keywords: | sqlite added |
---|---|
Summary: | One-off default migrations fail on tables with GeneratedField (on SQLite) → Adding a field with default crashes for models with GeneratedField on SQLite. |
comment:12 by , 13 months ago
Looks like felix already narrowed it down but here's a failing test case just in case someone can use it:
--- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -5801,6 +5801,36 @@ class OperationTests(OperationTestBase): def test_remove_generated_field_virtual(self): self._test_remove_generated_field(db_persist=False) + @skipUnlessDBFeature("supports_stored_generated_columns") + def test_add_field_after_generated_field(self): + app_label = "test_adfagf" + project_state = self.set_up_test_model(app_label) + operation_1 = migrations.AddField( + "Pony", + "generated", + models.GeneratedField( + expression=Value(1), + output_field=models.IntegerField(), + db_persist=True, + ), + ) + operation_2 = migrations.AddField( + "Pony", + "static", + models.IntegerField(default=2), + ) + new_state = project_state.clone() + operation_1.state_forwards(app_label, new_state) + with connection.schema_editor() as editor: + operation_1.database_forwards(app_label, editor, project_state, new_state) + project_state, new_state = new_state, new_state.clone() + operation_2.state_forwards(app_label, new_state) + with connection.schema_editor() as editor: + operation_2.database_forwards(app_label, editor, project_state, new_state) + pony = new_state.apps.get_model(app_label, "Pony").objects.create(weight=20) + self.assertEqual(pony.generated, 1) + self.assertEqual(pony.static, 2) + class SwappableOperationTests(OperationTestBase): """
comment:13 by , 13 months ago
I think it's enough to skip GeneratedField
s when remaking tables on SQLite:
-
django/db/backends/sqlite3/schema.py
diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index a8200a1a8c..713ef31437 100644
a b class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 106 106 # its values must be already quoted. 107 107 mapping = { 108 108 f.column: self.quote_name(f.column) 109 for f in model._meta.local_concrete_fields 109 for f in model._meta.local_concrete_fields if f.generated is False 110 110 } 111 111 # This maps field names (not columns) for things like unique_together 112 112 rename_mapping = {}
Sarah, would you like to prepare a patch?
follow-up: 15 comment:14 by , 13 months ago
I can tomorrow, if anyone wants to pick it up sooner that's also fine 🙂
comment:15 by , 13 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Replying to Sarah Boyce:
I can tomorrow, if anyone wants to pick it up sooner that's also fine 🙂
No rush, we can wait until tomorrow.
comment:16 by , 13 months ago
Has patch: | set |
---|
Hey Sarah, thank you for the report!
Could you please re-test with latest 5.0 rc1? The
GeneratedField
was changed so theoutput_field
is required, and when using that, I can't reproduce. I tried both with5.0rc1
and latest main.