Changing column to a foreign key in auto migration causes an exception

[gsavchuk]

Piccolo version 1.30
I tried to change the client field from
client = Integer(null=True)
to
client = ForeignKey(Client, target_column=Client.client_id, on_delete=OnDelete.restrict)

Generated the auto migration and it failed while running:

...
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/forwards.py", line 122, in run_forwards
    response = await manager.run()
               ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/forwards.py", line 99, in run
    return await self.run_migrations(app_config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/commands/forwards.py", line 83, in run_migrations
    await response.run()
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/auto/migration_manager.py", line 1010, in run
    await self._run_alter_columns(backwards=backwards)
  File "/usr/local/lib/python3.12/site-packages/piccolo/apps/migrations/auto/migration_manager.py", line 546, in _run_alter_columns
    constraint_name = await get_fk_constraint_name(
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/piccolo/query/constraints.py", line 43, in get_fk_constraint_name
    return constraints[0]["fk_constraint_name"]
           ~~~~~~~~~~~^^^
IndexError: list index out of range

This is because of the assumption that if a column is ForeignKey, than there is an existing FK constraint, which is not true in this case.

The generated auto-migration snippet:

    manager.alter_column(
        table_class_name="Temp",
        tablename="temp",
        column_name="client",
        db_column_name="client",
        params={
            "references": Client,
            "on_delete": OnDelete.restrict,
            "on_update": OnUpdate.cascade,
            "target_column": "client_id",
        },
        old_params={
            "references": None,
            "on_delete": None,
            "on_update": None,
            "target_column": None,
        },
        column_class=ForeignKey,
        old_column_class=Integer,
        schema=None,
    )

[sinisaos]

@gsavchuk Yes, this is a bug and I think you are right because a non-FK column does not have FK constraints and Piccolo is confused because it expects those constraints because it is assumed that we are only changing the action (e.g. ON DELETE etc.) of the FK column not column type (sorry if I'm wrong). This get_fk_constraint_name method was introduced in another bug where migrations don't register action changes (e.g. if we changed action in FK from OnDelete.restrict to OnDelete.cascade, that change wasn't reflected in the database). I made a small change from this code

piccolo/piccolo/query/constraints.py

Line 43 in 6ec3318

return constraints[0]["fk_constraint_name"]

to this

# if we change a column type from a non-FK column to
# an FK column, previous column type has no FK constraints
# and we just pass to allow the migration to continue
try:
    return constraints[0]["fk_constraint_name"]
except IndexError: ...
# or
if constraints:
    return constraints[0]["fk_constraint_name"]

and the migration was successful and the new column and constraints were added to the database. I don't know if this is correct but it works. @dantownsend