piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

applying `auto migration` does not rolled back the migration

Open Jacky56 opened this issue 4 months ago • 3 comments

Hello all,

Assume you have a auto migration which adds new columns:

1. manager.add_column(...)
2. manager.add_column(...)
3. manager.add_column(Some foreign key that doesn't exist as of yet will make the manager throw an error)
4. manager.add_column(...)

Now, what the piccolo.apps.migrations.auto.migration_manager.MigrationManager what will happen on the database:

  • add column 1
  • add column 2

now the manager will evaluate 3. but crash into an error, and will not rollback the 2 added columns on the database

piccolo version: piccolo[postgres]==1.24.2 cockroachDB version: Basic, Google Cloud v25.2.4

Jacky56 avatar Aug 24 '25 22:08 Jacky56

Ah ok, I need to test it. Are you running Postgres? Each migration should run inside a migration, and if something fails then the entire migration gets rolled back.

dantownsend avatar Aug 25 '25 18:08 dantownsend

The migration is executed in a transaction

https://github.com/piccolo-orm/piccolo/blob/5139ec0644675bc3b195d46de2d50571cfa9e4c7/piccolo/apps/migrations/auto/migration_manager.py#L982-L986

and if something goes wrong, the migration is rolled back. At least in my case everything works correctly. As in the example from the previous comment, if column 3 has an error, the migration transaction is rolled back and column 1 and column 2 are not added to the database. I think this is the correct behavior, at least in my case.

sinisaos avatar Aug 26 '25 04:08 sinisaos

hello @dantownsend @sinisaos

i forgotten to add the versions in the description:

piccolo version: piccolo[postgres]==1.24.2
cockroachDB version: Basic, Google Cloud v25.2.4

so the code mentioned by @sinisaos looks correct, so I can't give any assumptions from my end.

Here roughly the migration which did not rollback:

from piccolo.apps.migrations.auto.migration_manager import MigrationManager
from piccolo.columns.base import OnDelete
from piccolo.columns.base import OnUpdate
from piccolo.columns.column_types import ForeignKey
from piccolo.columns.column_types import Text
from piccolo.columns.column_types import Varchar
from piccolo.columns.indexes import IndexMethod
from piccolo.table import Table


# note that due to how migrations with multiple apps are ran https://github.com/piccolo-orm/piccolo/issues/1253
# This table from another app was not created yet
class Geolocation(Table, tablename="geolocation", schema=None):
    some_id= Text(
        default="",
        null=False,
        primary_key=True,
        unique=False,
        index=False,
        index_method=IndexMethod.btree,
        choices=None,
        db_column_name=None,
        secret=False,
    )


ID = "2025-03-06T20:49:20:965104"
VERSION = "1.21.0" # my local version during that time was using 1.21.0, but when i ran it (2025-08-24) its piccolo==1.24.2
DESCRIPTION = ""



async def forwards():
    manager = MigrationManager(
        migration_id=ID, app_name="user", description=DESCRIPTION
    )

    manager.add_column(
        table_class_name="User",
        tablename="user",
        column_name="column_1",
        db_column_name="column_1",
        column_class_name="Text",
        column_class=Text,
        params={
            "default": None,
            "null": True,
            "primary_key": False,
            "unique": False,
            "index": False,
            "index_method": IndexMethod.btree,
            "choices": None,
            "db_column_name": None,
            "secret": False,
        },
        schema=None,
    )

    manager.add_column(
        table_class_name="User",
        tablename="user",
        column_name="column_2",
        db_column_name="column_2",
        column_class_name="Text",
        column_class=Text,
        params={
            "default": None,
            "null": True,
            "primary_key": False,
            "unique": False,
            "index": False,
            "index_method": IndexMethod.btree,
            "choices": None,
            "db_column_name": None,
            "secret": False,
        },
        schema=None,
    )



    manager.add_column(
        table_class_name="User",
        tablename="user",
        column_name="geolocation",
        db_column_name="geolocation",
        column_class_name="ForeignKey",
        column_class=ForeignKey,
        params={
            "references": Geolocation,
            "on_delete": OnDelete.set_null,
            "on_update": OnUpdate.cascade,
            "target_column": None,
            "null": True,
            "primary_key": False,
            "unique": False,
            "index": False,
            "index_method": IndexMethod.btree,
            "choices": None,
            "db_column_name": None,
            "secret": False,
        },
        schema=None,
    )

    manager.add_column(
        table_class_name="User",
        tablename="user",
        column_name="column_4",
        db_column_name="column_4",
        column_class_name="Text",
        column_class=Text,
        params={
            "default": None,
            "null": True,
            "primary_key": False,
            "unique": False,
            "index": False,
            "index_method": IndexMethod.btree,
            "choices": None,
            "db_column_name": None,
            "secret": False,
        },
        schema=None,
    )

    return manager

Jacky56 avatar Aug 27 '25 17:08 Jacky56