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

id column pkey still added to migration/table when primary_key=True on other column #519

Open theelderbeever opened 2 years ago

theelderbeever commented 2 years ago

Defining your own primary key column used to prevent the generation of the id column. As of piccolo 0.74.x it appears to still generate the id column causing a multiple primary key error when applying the migration.

dantownsend commented 2 years ago

That's strange - I wonder if there's some kind of edge case. Do you mind sharing what the column name / type was for your custom primary key, and whether you defined multiple new columns on the table at the same time?

theelderbeever commented 2 years ago

Ah I figured out the initial issue... I am using a BaseTable class that inherits from Table in order to add utility methods. The base class was generating the id column.

So something like this.

class BaseTable(Table):
    @classmethod
    async def do_somthing(cls):
        ...

class MyTable(BaseTable):
    address = Text(primary_key=True)

With that it seems like inheritance might not work. I don't believe a Mixin will either since these are classmethods. Any thoughts on composability/extending a Base piccolo table?

theelderbeever commented 2 years ago

Further update the following appears to work but the order of the Mixins is critical (at least for some of the methods we wrote).

class BaseTable:
    @classmethod
    async def do_somthing(cls):
        ...

# This works
class MyTable(BaseTable, Table):
    address = Text(primary_key=True)

# This does not
class MyTable(Table, BaseTable):
    address = Text(primary_key=True)
dantownsend commented 2 years ago

OK, interesting - the docs around mixins need expanding upon to cover these details. We just have this currently.