Opened 10 years ago

Closed 9 years ago

#26114 closed Bug (fixed)

Removing Meta.db_table generates migration with wrong rename in comment (None)

Reported by: Daniel Owned by: PREMANAND
Component: Migrations Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

# myapp.models.py

#Old model
class MyModel(models.Model):
    ...
    class Meta:
        db_table = 'legacy_table'

#New model
class MyModel(models.Model):
    ...

# makemigrations output:
- Rename table for mymodel to None

# sqlmigrate output:
BEGIN;
--
-- Rename table for mymodel to None
--
RENAME TABLE `legacy_table` TO `myapp_mymodel`;

COMMIT;


Change History (7)

comment:1 by Tim Graham, 10 years ago

Summary: Removing Meta.db_table generates migration with wrong rename (None)Removing Meta.db_table generates migration with wrong rename in comment (None)
Triage Stage: UnreviewedAccepted

I think we don't have the needed information to put the actual name in the comment, but perhaps we could replace None with something like (default).

comment:2 by PREMANAND, 10 years ago

Owner: changed from nobody to PREMANAND
Status: newassigned

comment:3 by PREMANAND, 10 years ago

I'm thinking the following, please review and provide your comments.

When someone provides "None" as the new table name, we won't detect that as a change and just ignore it.

https://github.com/django/django/blob/fb4272f0e6bbdaa3e19ed5fde59fdb5ab5a33baf/django/db/migrations/autodetector.py#L962

This is the new change that works.

if old_db_table_name != new_db_table_name and new_db_table_name != None:
                self.add_operation(
                    app_label,
                    operations.AlterModelTable(
                        name=model_name,
                        table=new_db_table_name,
                    )
                )

We don't have the same problem in the initial creation of the table if the table name is "None". It defaults to the app name as normal.

Last edited 10 years ago by PREMANAND (previous) (diff)

comment:4 by Tim Graham, 10 years ago

The change I proposed in comment 1 is about modifying AlterModelTable.describe(). The autodetector shouldn't be changed like that because then a migration changing the table name back to the default wouldn't be created.

comment:5 by PREMANAND, 10 years ago

Last edited 9 years ago by Tim Graham (previous) (diff)

comment:6 by Tim Graham, 9 years ago

Has patch: set

comment:7 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 5da7e3f:

Fixed #26114 -- Fixed AlterModelTable.describe() if db_table is None.

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