Opened 9 years ago

Closed 8 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, 9 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, 8 years ago

Owner: changed from nobody to PREMANAND
Status: newassigned

comment:3 by PREMANAND, 8 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 8 years ago by PREMANAND (previous) (diff)

comment:4 by Tim Graham, 8 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, 8 years ago

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

comment:6 by Tim Graham, 8 years ago

Has patch: set

comment:7 by Tim Graham <timograham@…>, 8 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