Foreign Keys that reference non pk self columns

[stronk7]

This is a followup of #393.

I've been using FKs pointing to other tables UKs perfectly. But now, I've the typical parent-child relationship that uses a "self" UK. Sort of:

class Test(Table, tablename="test"):
    id = Serial(primary_key=True)
    uk_id = Varchar(length=255, null=False, unique=True)
    child_id = ForeignKey("self", null=True, index=True, target_column="uk_id")

The definition above leads to the creation of an apparently good migration file, but when I run migrations, it fails with "piccolo/query/base.py", line 387, in postgres_ddl
raise NotImplementedError".

I also have tried "self.uk_id" (string), uk_id (property name), ... but in all those cases the creation of migration file doesn't work. Also have looked to some example in the docs and issues about how to make a FK to point to a "self" UK column (PK ones work ok, and also "remote" UK ones work ok) with no luck.

So, is there any "trick" to get those "self" + "uk" ones working? TIA!

[stronk7]

I think I've found the difference between creating FKs pointing to another table UK and to "self" UK.

1. When a FK pointing to another table UK is created, for example:

class Origin(Table, tablename="origin"):
    id = Serial(primary_key=True)
    uk_id = Varchar(length=255, null=False, unique=True)

class Test(Table, tablename="test"):
    id = Serial(primary_key=True)
    child_id = ForeignKey(Origin, null=True, index=True, target_column="uk_id")

The migration file includes, before the forwards() function, a section defining the Origin table related columns (id and uk_id). And then, the migration works.

2. But, when the FK is pointing to "self":

class Test(Table, tablename="test"):
    id = Serial(primary_key=True)
    uk_id = Varchar(length=255, null=False, unique=True)
    child_id = ForeignKey("self", null=True, index=True, target_column="uk_id")

Then the migration file is missing that section defining the "self" (Test) table related columns and the migration ends with No matching column found with name error (with --trace enabled).

So, I've edited the migration file manually, adding a Test table declaration, just containing the uk_id column specification and then have edited the forwards() method, moving to use that table declaration and, bump, everything has worked ok so far:

class Test(Table, tablename="test", schema=None):    #    <== This is the missing declaration in the "self" case.
    uk_id = Varchar(
        length=255,
        default=None,
        null=False,
        unique=True,
        ...
        ...
    )
...
...
async def forwards():
...
...
    manager.add_column(
        table_class_name="Test",
        tablename="test",
        column_name="child_id",
        db_column_name="child_id",
        column_class_name="ForeignKey",
        column_class=ForeignKey,
        params={
            "references": Test,      #    <== had to change this too, from "self" (string) to Test (the class).
            "on_delete": OnDelete.no_action,
            "on_update": OnUpdate.no_action,
            "target_column": "uk_id",
            ....
        },
        schema=None,
    )

So, should this be considered a bug and that section defining the involved target columns should be added in the "self" case?

[sinisaos]

@stronk7 I cannot reproduce your issue. In my case everything works fine. With your table definition

class Test(Table, tablename="test"):
    id = Serial(primary_key=True)
    uk_id = Varchar(length=255, null=False, unique=True)
    child_id = ForeignKey("self", null=True, index=True, target_column="uk_id")

I can create and run the migration.

[stronk7]

Thanks for trying, @sinisaos

My fault, I forgot / didn't explain it clearly. I already have the Test table created and the initial migration generated:

class Test(Table, tablename="test"):
    id = Serial(primary_key=True)
    uk_id = Varchar(length=255, null=False, unique=True)

And it's when I try to add the "self" FK to it:

class Test(Table, tablename="test"):
    id = Serial(primary_key=True)
    uk_id = Varchar(length=255, null=False, unique=True)
    child_id = ForeignKey("self", null=True, index=True, target_column="uk_id")

is when the generated migration does not include that "preface" information (that I detailed in my second comment) needed to run the 2nd migration properly.

As said, it works for "remote" FKs (the "preface" is added to the new migration), but not for "self" ones (the "preface" is missing).

Ciao :-)

[sinisaos]

@stronk7 In that case, it doesn't work for me either. It's strange that everything works on creating the table but not on alter table even though the manager.add_column() methods are exactly the same in both cases

So, should this be considered a bug and that section defining the involved target columns should be added in the "self" case?

I don't know if it's a bug or an edge case, but I don't think target_column serves this purpose. target_column is used for non-primary key references to another table. I think the self-referential foreign key in Piccolo is bound to the primary key of the table, not the target_column. If you remove the target_column from the table definition, everything works and the constraints are set to primary key of the Test table. If you still want the uk_id to be the FK constraint, you can make your own workaround, or you can simply import the Test table into the migration file and use it as a reference like this

from your_app_path.tables import Test

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

  manager.add_column(
      ...
      params={
          "references": Test, 

[stronk7]

Well, when the FK points to a remote (another table) UK, then it works perfectly and the information from the Origin table needed to achieve the migration execution is added correctly (as a preamble in the migration file).

I'd expect the FK pointing to a self (same table) UK to behave the same, and lead to working migrations. Cannot see why it makes any difference and they just fail because of the lack of that information. I'm not sure it's an edge case, for me it's exactly the same than the "remote" UK one.

In any case, yes, I can workaround it by adding the preamble information by hand, or by importing the table, although... the 2nd could be not future-proof if at some time some of the columns go away (I think that's the reason the migrations always write "current" correct preambles instead of importing, because the later are not good snapshots.

Ciao :-)