Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#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 Natalia Bidart, 13 months ago

Resolution: worksforme
Status: newclosed

Hey Sarah, thank you for the report!

Could you please re-test with latest 5.0 rc1? The GeneratedField was changed so the output_field is required, and when using that, I can't reproduce. I tried both with 5.0rc1 and latest main.

comment:2 by Sarah Boyce, 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 David Sanders, 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 David Sanders, 13 months ago

Triage Stage: UnreviewedAccepted

comment:5 by David Sanders, 13 months ago

Resolution: worksforme
Status: closednew

comment:6 by Natalia Bidart, 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 Natalia Bidart, 13 months ago

Component: UncategorizedDatabase layer (models, ORM)
Type: UncategorizedBug

comment:8 by Mariusz Felisiak, 13 months ago

Severity: NormalRelease blocker

comment:9 by Natalia Bidart, 13 months ago

Severity: Release blockerNormal

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 Natalia Bidart, 13 months ago

Severity: NormalRelease blocker

comment:11 by Mariusz Felisiak, 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 David Sanders, 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 Mariusz Felisiak, 13 months ago

I think it's enough to skip GeneratedFields 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):  
    106106        # its values must be already quoted.
    107107        mapping = {
    108108            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
    110110        }
    111111        # This maps field names (not columns) for things like unique_together
    112112        rename_mapping = {}

Sarah, would you like to prepare a patch?

comment:14 by Sarah Boyce, 13 months ago

I can tomorrow, if anyone wants to pick it up sooner that's also fine 🙂

in reply to:  14 comment:15 by Mariusz Felisiak, 13 months ago

Owner: changed from nobody to Sarah Boyce
Status: newassigned

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 Sarah Boyce, 13 months ago

Has patch: set

comment:17 by Mariusz Felisiak, 13 months ago

Triage Stage: AcceptedReady for checkin

comment:18 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

Resolution: fixed
Status: assignedclosed

In 828082da:

Fixed #34984 -- Skipped GeneratedFields when remaking tables on SQLite.

Regression in f333e3513e8bdf5ffeb6eeb63021c230082e6f95.t

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>
Co-authored-by: David Sanders <shang.xiao.sanders@…>

comment:19 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

In 0c6ca522:

[5.0.x] Fixed #34984 -- Skipped GeneratedFields when remaking tables on SQLite.

Regression in f333e3513e8bdf5ffeb6eeb63021c230082e6f95.t

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>
Co-authored-by: David Sanders <shang.xiao.sanders@…>

Backport of 828082dad954e87d09a99b53424e6faa1860ccc7 from main

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