coleifer / peewee

a small, expressive orm -- supports postgresql, mysql, sqlite and cockroachdb
http://docs.peewee-orm.com/
MIT License
11.03k stars 1.37k forks source link

Correct way to migrate a table to update a ForeignKeyConstraint with `ON DELETE CASCADE` #2902

Closed nitesh-jaswal closed 3 months ago

nitesh-jaswal commented 3 months ago

Summary

Hey @coleifer! This is something me and my team have been struggling with for some time. We have table which is added as a Foreign Key to many other tables. However, there were no referential actions like ON DELETE CASCADE added to these tables which was an oversight on our ed.

We want to update the Foreign Key Constraints on these other tables and add the referential action ON DELETE CASCADE. However, whenever we use the migrator from playhouse we notice there is a loss of data. The schema for the tables is indeed updated but there is no data in the newly created tables.

Context

class Service:
    id: Union[str, int, AutoField] = CharField(max_length=255, null=False, primary_key=True, default=lambda: str(uuid4()))
    messages: Union[list, JsonField] = JsonField(default=lambda: [], null=False)
    ... # Other fields

class ServiceExecution(AbstractTable):
    id: Union[int, AutoField] = AutoField()
    query_service: Union[QueryService, ForeignKeyField] = ForeignKeyField(model=QueryService, null=True)
    ... # Other fields

class ServiceExecutionPartition(AbstractTable):
    id: Union[int, AutoField] = AutoField()
    execution: Union[QWExecution, ForeignKeyField] = ForeignKeyField(model=QWExecution)
    query_service: Union[QueryService, ForeignKeyField] = ForeignKeyField(model=QueryService, null=True)

Our objective is to add ON DELETE CASCADE without causing a loss of data in any of our tables. A sample of the script we tried using.

from playhouse.migrate import MySQLMigrator, operation, Entity
from utility.databases import database

...

class CustomMigrator(MySQLMigrator):
    @operation
    def alter_column_type(self, table, column, field, cast=None):
        if cast is not None:
            raise ValueError('alter_column_type() does not support cast with '
                             'MySQL.')
        ctx = self.make_context()
        return (self
                ._alter_table(ctx, table)
                .literal(' MODIFY ')
                .sql(Entity(column))
                .literal(' ')
                .sql(field.ddl(ctx))

def add_cascade_delete(migrator: MySQLMigrator):
    with migrator.database.transaction():
        # Add a new column with CASCADE delete constraint
        migrator.drop_column(ServiceExecutionPartition._meta.table_name, ServiceExecutionPartition.execution.column_name).run()
        migrator.add_column(
           ServiceExecutionPartition._meta.table_name,
           ServiceExecutionPartition.execution.column_name,
           ForeignKeyField(model=ServiceExecution, field=ServiceExecution.id, null=True, on_delete='CASCADE')).run()

        migrator.drop_column(ServiceExecutionPartition._meta.table_name, ServiceExecutionPartition.query_service.column_name).run()
        migrator.add_column(
           ServiceExecutionPartition._meta.table_name,
           ServiceExecutionPartition.query_service.column_name,
           ForeignKeyField(model=Service, field=Service.id, null=True, on_delete='CASCADE')).run()

        migrator.drop_column(ServiceExecution._meta.table_name, ServiceExecution.query_service.column_name).run()
        migrator.add_column(
           ServiceExecution._meta.table_name,
           ServiceExecution.query_service.column_name,
           ForeignKeyField(model=Service, null=True, field=Service.id, on_delete='CASCADE')).run()

if __name__ == "__main__":
    migrator = CustomMigrator(database)
    add_cascade_delete(migrator)

Side Note: We hoped doing this as a transaction would rollback any changes in case there was some error in our script. However, this did not happen either.

What would the recommended way of doing this would be? Any help here is highly appreciated. Thanks!

coleifer commented 3 months ago

I'm pretty sure MySQL doesn't support transactional DDL, see https://dev.mysql.com/doc/refman/8.0/en/atomic-ddl.html

Regarding your question, I would just write the SQL out and run it rather than trying to wrap it up in anything too fancy. It looks like you're dropping then adding the column, which will obviously lose data. This is what you want to run:

ALTER TABLE service_execution DROP FOREIGN KEY foreign_key_name;
ALTER TABLE service_execution ADD CONSTRAINT foreign_key_name
    FOREIGN KEY (foreign_key_column)
    REFERENCES dest_table(id) 
    ON DELETE CASCADE;

You can wrap this up in a function to make it reusable if you wish, e.g.

def get_constraint_name(field):
    curs = db.execute_sql('select constraint_name '
                          'from information_schema.key_column_usage '
                          'where table_name=%s and column_name=%s',
                          (field.model._meta.table_name, field.column_name))
    return curs.fetchone()[0]

def add_on_delete_cascade(field):
    cons = get_constraint_name(field)
    tbl = field.model._meta.table_name
    col = field.column_name
    db.execute_sql('alter table `%s` drop foreign key `%s`' % (tbl, cons))
    db.execute_sql('alter table %s add constraint %s '
                   'foreign key (%s) references %s(%s) '
                   'on delete cascade' %
                   (tbl, cons, col, field.rel_model._meta.table_name,
                    field.rel_field.column_name))