Opened 7 months ago

Closed 7 months ago

Last modified 7 months ago

#34946 closed Bug (fixed)

Adding a field with default and db_default drops database level DEFAULT

Reported by: Simon Charette Owned by: Simon Charette
Component: Migrations Version: 5.0
Severity: Release blocker Keywords: sql, default, migrations
Cc: Paolo Melchiorre Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The documentation for Field.db_default states

If both db_default and Field.default are set, default will take precedence when creating instances in Python code. db_default will still be set at the database level and will be used when inserting rows outside of the ORM or when adding a new field in a migration.

However adding a field with both results in the following SQL

ALTER TABLE "foo" ADD COLUMN "bar" integer DEFAULT 42 NOT NULL;
ALTER TABLE "foo" ALTER COLUMN "bar" DROP DEFAULT;

The DEFAULT should never be dropped if the field has a db_default defined.

The following patch reproduces and demonstrate why the test that intended to cover this case didn't catch it.

  • tests/migrations/test_operations.py

    diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
    index d58da8b7ac..57a9086c19 100644
    a b def test_add_field_both_defaults(self):  
    16921692        field = new_state.models[app_label, "pony"].fields["height"]
    16931693        self.assertEqual(field.default, 3)
    16941694        self.assertEqual(field.db_default, Value(4))
    1695         project_state.apps.get_model(app_label, "pony").objects.create(weight=4)
     1695        pre_pony_pk = (
     1696            project_state.apps.get_model(app_label, "pony").objects.create(weight=4).pk
     1697        )
    16961698        self.assertColumnNotExists(table_name, "height")
    16971699        # Add field.
    16981700        with connection.schema_editor() as editor:
    16991701            operation.database_forwards(app_label, editor, project_state, new_state)
    17001702        self.assertColumnExists(table_name, "height")
     1703        post_pony_pk = (
     1704            project_state.apps.get_model(app_label, "pony").objects.create(weight=10).pk
     1705        )
    17011706        new_model = new_state.apps.get_model(app_label, "pony")
    1702         old_pony = new_model.objects.get()
    1703         self.assertEqual(old_pony.height, 4)
     1707        pre_pony = new_model.objects.get(pk=pre_pony_pk)
     1708        self.assertEqual(pre_pony.height, 4)
     1709        post_pony = new_model.objects.get(pk=post_pony_pk)
     1710        self.assertEqual(post_pony.height, 4)
    17041711        new_pony = new_model.objects.create(weight=5)
    17051712        if not connection.features.can_return_columns_from_insert:
    17061713            new_pony.refresh_from_db()
  • tests/schema/tests.py

    diff --git a/tests/schema/tests.py b/tests/schema/tests.py
    index 20a00e29f7..80fdead190 100644
    a b class Meta:  
    22572257        columns = self.column_classes(AuthorDbDefault)
    22582258        self.assertEqual(columns["renamed_year"][1].default, "1985")
    22592259
     2260    @isolate_apps("schema")
     2261    def test_add_field_both_defaults_preserves_db_default(self):
     2262        class Author(Model):
     2263            class Meta:
     2264                app_label = "schema"
     2265
     2266        #self.isolated_local_models = [Author]
     2267        with connection.schema_editor() as editor:
     2268            editor.create_model(Author)
     2269
     2270        field = IntegerField(default=1985, db_default=1988)
     2271        field.set_attributes_from_name("birth_year")
     2272        field.model = Author
     2273        with connection.schema_editor() as editor:
     2274            editor.add_field(Author, field)
     2275        columns = self.column_classes(Author)
     2276        self.assertEqual(columns["birth_year"][1].default, "1988")
     2277
    22602278    @skipUnlessDBFeature(
    22612279        "supports_column_check_constraints", "can_introspect_check_constraints"
    22622280    )

Change History (7)

comment:1 by Simon Charette, 7 months ago

Has patch: set

comment:2 by Mariusz Felisiak, 7 months ago

Owner: changed from nobody to Simon Charette
Triage Stage: UnreviewedAccepted

Thanks!

comment:3 by Paolo Melchiorre, 7 months ago

Cc: Paolo Melchiorre added
Keywords: sql default migrations added

in reply to:  description comment:4 by Natalia Bidart, 7 months ago

Replying to Simon Charette:

However adding a field with both results in the following SQL

ALTER TABLE "foo" ADD COLUMN "bar" integer DEFAULT 42 NOT NULL;
ALTER TABLE "foo" ALTER COLUMN "bar" DROP DEFAULT;

Thank you Simon for the report. I've been trying to reproduce the issue exactly as described to progress with the PR review. I've created a test app with a Foo model with a dummy name field, and then I've added a bar integer field with both default and db_default. I can't get the SQL that you reported, so far I'm only obtaining a SQL that would create an intermediary table. I think I'm missing something to craft the proper reproducer:

BEGIN;
--
-- Add field bar to foo
--
CREATE TABLE "new__ticket_34946_foo" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "bar" integer DEFAULT 42 NOT NULL, "name" varchar(10) DEFAULT 'dbdef' NOT NULL);
INSERT INTO "new__ticket_34946_foo" ("id", "name") SELECT "id", "name" FROM "ticket_34946_foo";
DROP TABLE "ticket_34946_foo";
ALTER TABLE "new__ticket_34946_foo" RENAME TO "ticket_34946_foo";
COMMIT;

I've also tried this with a Foo model with no other field than bar, same result. Could you help me understand the steps to reproduce the error as reported? Thank you!

comment:5 by Simon Charette, 7 months ago

Hello Natalia!

SQLite doesn't support alteration such as DROP DEFAULT so it's not affected by this issue, you should be able to reproduce with Postgres or MySQL.

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

Resolution: fixed
Status: assignedclosed

In 8a28e983:

Fixed #34946 -- Preserved db_default on combined default field addition.

Regression in 7414704e88d73dafbcfbb85f9bc54cb6111439d3.

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

In 0265eaa5:

[5.0.x] Fixed #34946 -- Preserved db_default on combined default field addition.

Regression in 7414704e88d73dafbcfbb85f9bc54cb6111439d3.
Backport of 8a28e983df091d94eaba77cb82fbe3ef60a80799 from main

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