piccolo-orm / piccolo

A fast, user friendly ORM and query builder which supports asyncio.
https://piccolo-orm.com/
MIT License
1.45k stars 91 forks source link

Array type migrations fail to import all required items #1076

Open Skelmis opened 2 months ago

Skelmis commented 2 months ago

When moving Array base column types, it looks like the migration engine fails to correctly import the relevant columns.

Migration one, works fine:

from piccolo.apps.migrations.auto.migration_manager import MigrationManager
from piccolo.columns.column_types import Array
from piccolo.columns.column_types import Boolean
from piccolo.columns.column_types import Serial
from piccolo.columns.indexes import IndexMethod

ID = "2024-09-14T20:13:48:245769"
VERSION = "1.17.0"
DESCRIPTION = ""

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

    manager.add_column(
        table_class_name="Project",
        tablename="project",
        column_name="is_public",
        db_column_name="is_public",
        column_class_name="Boolean",
        column_class=Boolean,
        params={
            "default": False,
            "null": False,
            "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="Project",
        tablename="project",
        column_name="other_users",
        db_column_name="other_users",
        column_class_name="Array",
        column_class=Array,
        params={
            "base_column": Serial(
                null=False,
                primary_key=False,
                unique=False,
                index=False,
                index_method=IndexMethod.btree,
                choices=None,
                db_column_name=None,
                secret=False,
            ),
            "default": list,
            "null": False,
            "primary_key": False,
            "unique": False,
            "index": False,
            "index_method": IndexMethod.btree,
            "choices": None,
            "db_column_name": None,
            "secret": False,
        },
        schema=None,
    )

    return manager

Then when attempting to move to Integer from Serial:

from piccolo.apps.migrations.auto.migration_manager import MigrationManager
from piccolo.columns.column_types import Array
from piccolo.columns.column_types import Serial
from piccolo.columns.indexes import IndexMethod

ID = "2024-09-14T21:09:56:790331"
VERSION = "1.17.0"
DESCRIPTION = ""

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

    manager.alter_column(
        table_class_name="Project",
        tablename="project",
        column_name="other_users",
        db_column_name="other_users",
        params={
            "base_column": Integer(
                default=0,
                null=False,
                primary_key=False,
                unique=False,
                index=False,
                index_method=IndexMethod.btree,
                choices=None,
                db_column_name=None,
                secret=False,
            )
        },
        old_params={
            "base_column": Serial(
                null=False,
                primary_key=False,
                unique=False,
                index=False,
                index_method=IndexMethod.btree,
                choices=None,
                db_column_name=None,
                secret=False,
            )
        },
        column_class=Array,
        old_column_class=Array,
        schema=None,
    )

    return manager

Where in this migration Piccolo fails to import the Integer column type which results in a failure which requires manual intervention.

The command failed.
name 'Integer' is not defined
Traceback (most recent call last):
  File "/home/skelmis/.cache/pypoetry/virtualenvs/hermes-nZAhPr0C-py3.10/lib/python3.10/site-packages/targ/__init__.py", line 449, in run
    command.call_with(arg_class)
  File "/home/skelmis/.cache/pypoetry/virtualenvs/hermes-nZAhPr0C-py3.10/lib/python3.10/site-packages/targ/__init__.py", line 230, in call_with
    asyncio.run(self.command(**cleaned_kwargs))
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "uvloop/loop.pyx", line 1517, in uvloop.loop.Loop.run_until_complete
  File "/home/skelmis/.cache/pypoetry/virtualenvs/hermes-nZAhPr0C-py3.10/lib/python3.10/site-packages/piccolo/apps/migrations/commands/forwards.py", line 162, in forwards
    response = await run_forwards(
  File "/home/skelmis/.cache/pypoetry/virtualenvs/hermes-nZAhPr0C-py3.10/lib/python3.10/site-packages/piccolo/apps/migrations/commands/forwards.py", line 123, in run_forwards
    response = await manager.run()
  File "/home/skelmis/.cache/pypoetry/virtualenvs/hermes-nZAhPr0C-py3.10/lib/python3.10/site-packages/piccolo/apps/migrations/commands/forwards.py", line 100, in run
    return await self.run_migrations(app_config)
  File "/home/skelmis/.cache/pypoetry/virtualenvs/hermes-nZAhPr0C-py3.10/lib/python3.10/site-packages/piccolo/apps/migrations/commands/forwards.py", line 76, in run_migrations
    response = await migration_module.forwards()
  File "/home/skelmis/Code/Hermes/home/piccolo_migrations/home_2024_09_14t21_09_56_790331.py", line 23, in forwards
    "base_column": Integer(
NameError: name 'Integer' is not defined
dantownsend commented 2 months ago

Ah, interesting.

Array columns are tricky, and have caused lots of weirdness in the past.

It should hopefully be a simple fix - I'll try and recreate it locally.